diff --git a/tools/aconfig/src/aconfig.rs b/tools/aconfig/src/aconfig.rs deleted file mode 100644 index 5e7c861956..0000000000 --- a/tools/aconfig/src/aconfig.rs +++ /dev/null @@ -1,306 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -use anyhow::{anyhow, bail, Context, Error, Result}; -use protobuf::{Enum, EnumOrUnknown}; -use serde::{Deserialize, Serialize}; - -use crate::cache::{Cache, Item, Tracepoint}; -use crate::protos::{ - ProtoFlagDeclaration, ProtoFlagDeclarations, ProtoFlagPermission, ProtoFlagState, - ProtoFlagValue, ProtoFlagValues, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint, -}; - -#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)] -pub enum FlagState { - Enabled, - Disabled, -} - -impl TryFrom> for FlagState { - type Error = Error; - - fn try_from(proto: EnumOrUnknown) -> Result { - 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())), - } - } -} - -impl From for ProtoFlagState { - fn from(state: FlagState) -> Self { - match state { - FlagState::Enabled => ProtoFlagState::ENABLED, - FlagState::Disabled => ProtoFlagState::DISABLED, - } - } -} - -#[derive(Debug, PartialEq, Eq, Serialize, Deserialize, Clone, Copy)] -pub enum Permission { - ReadOnly, - ReadWrite, -} - -impl TryFrom> for Permission { - type Error = Error; - - fn try_from(proto: EnumOrUnknown) -> Result { - match ProtoFlagPermission::from_i32(proto.value()) { - Some(ProtoFlagPermission::READ_ONLY) => Ok(Permission::ReadOnly), - Some(ProtoFlagPermission::READ_WRITE) => Ok(Permission::ReadWrite), - None => Err(anyhow!("unknown permission enum value {}", proto.value())), - } - } -} - -impl From for ProtoFlagPermission { - fn from(permission: Permission) -> Self { - match permission { - Permission::ReadOnly => ProtoFlagPermission::READ_ONLY, - Permission::ReadWrite => ProtoFlagPermission::READ_WRITE, - } - } -} - -#[derive(Debug, PartialEq, Eq)] -pub struct FlagDeclaration { - pub name: String, - pub namespace: String, - pub description: String, -} - -impl FlagDeclaration { - #[allow(dead_code)] // only used in unit tests - pub fn try_from_text_proto(text_proto: &str) -> Result { - let proto: ProtoFlagDeclaration = crate::protos::try_from_text_proto(text_proto) - .with_context(|| text_proto.to_owned())?; - proto.try_into() - } -} - -impl TryFrom for FlagDeclaration { - type Error = Error; - - fn try_from(proto: ProtoFlagDeclaration) -> Result { - let Some(name) = proto.name else { - bail!("missing 'name' field"); - }; - let Some(namespace) = proto.namespace else { - bail!("missing 'namespace' field"); - }; - let Some(description) = proto.description else { - bail!("missing 'description' field"); - }; - Ok(FlagDeclaration { name, namespace, description }) - } -} - -#[derive(Debug, PartialEq, Eq)] -pub struct FlagDeclarations { - pub package: String, - pub flags: Vec, -} - -impl FlagDeclarations { - pub fn try_from_text_proto(text_proto: &str) -> Result { - let proto: ProtoFlagDeclarations = crate::protos::try_from_text_proto(text_proto) - .with_context(|| text_proto.to_owned())?; - let Some(package) = proto.package else { - bail!("missing 'package' field"); - }; - let mut flags = vec![]; - for proto_flag in proto.flag.into_iter() { - flags.push(proto_flag.try_into()?); - } - Ok(FlagDeclarations { package, flags }) - } -} - -#[derive(Debug, PartialEq, Eq)] -pub struct FlagValue { - pub package: String, - pub name: String, - pub state: FlagState, - pub permission: Permission, -} - -impl FlagValue { - #[allow(dead_code)] // only used in unit tests - pub fn try_from_text_proto(text_proto: &str) -> Result { - 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> { - 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 for FlagValue { - type Error = Error; - - fn try_from(proto: ProtoFlagValue) -> Result { - let Some(package) = proto.package else { - bail!("missing 'package' field"); - }; - let Some(name) = proto.name else { - bail!("missing 'name' field"); - }; - let Some(proto_state) = proto.state else { - bail!("missing 'state' field"); - }; - let state = proto_state.try_into()?; - let Some(proto_permission) = proto.permission else { - bail!("missing 'permission' field"); - }; - let permission = proto_permission.try_into()?; - Ok(FlagValue { package, name, state, permission }) - } -} - -impl From for ProtoParsedFlags { - fn from(cache: Cache) -> Self { - let mut proto = ProtoParsedFlags::new(); - for item in cache.into_iter() { - proto.parsed_flag.push(item.into()); - } - proto - } -} - -impl From for ProtoParsedFlag { - fn from(item: Item) -> Self { - let mut proto = crate::protos::ProtoParsedFlag::new(); - proto.set_package(item.package.to_owned()); - proto.set_name(item.name.clone()); - proto.set_namespace(item.namespace.clone()); - proto.set_description(item.description.clone()); - proto.set_state(item.state.into()); - proto.set_permission(item.permission.into()); - for trace in item.trace.into_iter() { - proto.trace.push(trace.into()); - } - proto - } -} - -impl From for ProtoTracepoint { - fn from(tracepoint: Tracepoint) -> Self { - let mut proto = ProtoTracepoint::new(); - proto.set_source(format!("{}", tracepoint.source)); - proto.set_state(tracepoint.state.into()); - proto.set_permission(tracepoint.permission.into()); - proto - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_flag_try_from_text_proto() { - let expected = FlagDeclaration { - name: "1234".to_owned(), - namespace: "ns".to_owned(), - description: "Description of the flag".to_owned(), - }; - - let s = r#" - name: "1234" - namespace: "ns" - description: "Description of the flag" - "#; - let actual = FlagDeclaration::try_from_text_proto(s).unwrap(); - - assert_eq!(expected, actual); - } - - #[test] - fn test_flag_try_from_text_proto_bad_input() { - let s = r#" - name: "a" - "#; - let error = FlagDeclaration::try_from_text_proto(s).unwrap_err(); - assert!(format!("{:?}", error).contains("Message not initialized")); - - let s = r#" - description: "Description of the flag" - "#; - let error = FlagDeclaration::try_from_text_proto(s).unwrap_err(); - assert!(format!("{:?}", error).contains("Message not initialized")); - } - - #[test] - fn test_package_try_from_text_proto() { - let expected = FlagDeclarations { - package: "com.example".to_owned(), - flags: vec![ - FlagDeclaration { - name: "a".to_owned(), - namespace: "ns".to_owned(), - description: "A".to_owned(), - }, - FlagDeclaration { - name: "b".to_owned(), - namespace: "ns".to_owned(), - description: "B".to_owned(), - }, - ], - }; - - let s = r#" - package: "com.example" - flag { - name: "a" - namespace: "ns" - description: "A" - } - flag { - name: "b" - namespace: "ns" - description: "B" - } - "#; - let actual = FlagDeclarations::try_from_text_proto(s).unwrap(); - - assert_eq!(expected, actual); - } - - #[test] - fn test_flag_declaration_try_from_text_proto_list() { - let expected = FlagValue { - package: "com.example".to_owned(), - name: "1234".to_owned(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }; - - let s = r#" - package: "com.example" - name: "1234" - state: ENABLED - permission: READ_ONLY - "#; - let actual = FlagValue::try_from_text_proto(s).unwrap(); - - assert_eq!(expected, actual); - } -} diff --git a/tools/aconfig/src/cache.rs b/tools/aconfig/src/cache.rs deleted file mode 100644 index dd54480fd0..0000000000 --- a/tools/aconfig/src/cache.rs +++ /dev/null @@ -1,388 +0,0 @@ -/* - * Copyright (C) 2023 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -use anyhow::{bail, ensure, Result}; -use serde::{Deserialize, Serialize}; -use std::io::{Read, Write}; - -use crate::aconfig::{FlagDeclaration, FlagState, FlagValue, Permission}; -use crate::codegen; -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, - pub state: FlagState, - pub permission: Permission, -} - -#[derive(Serialize, Deserialize, Debug)] -pub struct Item { - // TODO: duplicating the Cache.package as Item.package makes the internal representation - // closer to the proto message `parsed_flag`; hopefully this will enable us to replace the Item - // struct and use a newtype instead once aconfig has matured. Until then, package should - // really be a Cow. - pub package: String, - pub name: String, - pub namespace: String, - pub description: String, - pub state: FlagState, - pub permission: Permission, - pub trace: Vec, -} - -#[derive(Serialize, Deserialize, Debug)] -pub struct Cache { - package: String, - 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, f: F) -> bool -where - F: FnMut(&'a T) -> K, - K: PartialOrd, -{ - let mut last: Option = 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 { - 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 { - self.items.iter() - } - - pub fn into_iter(self) -> impl Iterator { - self.items.into_iter() - } - - pub fn package(&self) -> &str { - debug_assert!(!self.package.is_empty()); - &self.package - } -} - -#[derive(Debug)] -pub struct CacheBuilder { - cache: Cache, -} - -impl CacheBuilder { - pub fn new(package: String) -> Result { - ensure!(codegen::is_valid_package_ident(&package), "bad package"); - let cache = Cache { package, items: vec![] }; - Ok(CacheBuilder { cache }) - } - - pub fn add_flag_declaration( - &mut self, - source: Source, - declaration: FlagDeclaration, - ) -> Result<&mut CacheBuilder> { - ensure!(codegen::is_valid_name_ident(&declaration.name), "bad flag name"); - ensure!(codegen::is_valid_name_ident(&declaration.namespace), "bad namespace"); - ensure!(!declaration.description.is_empty(), "empty flag description"); - ensure!( - self.cache.items.iter().all(|item| item.name != declaration.name), - "failed to declare flag {} from {}: flag already declared", - declaration.name, - source - ); - self.cache.items.push(Item { - package: self.cache.package.clone(), - name: declaration.name.clone(), - namespace: declaration.namespace.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(self) - } - - pub fn add_flag_value( - &mut self, - source: Source, - value: FlagValue, - ) -> Result<&mut CacheBuilder> { - 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 {}", - value.package, - value.name, - source, - self.cache.package - ); - 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.package, value.name, source); - }; - existing_item.state = value.state; - existing_item.permission = value.permission; - existing_item.trace.push(Tracepoint { - source, - state: value.state, - permission: value.permission, - }); - Ok(self) - } - - pub fn build(mut self) -> Cache { - self.cache.items.sort_by_cached_key(|item| item.name.clone()); - self.cache - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_add_flag_declaration() { - let mut builder = CacheBuilder::new("com.example".to_string()).unwrap(); - builder - .add_flag_declaration( - Source::File("first.txt".to_string()), - FlagDeclaration { - name: "foo".to_string(), - namespace: "ns".to_string(), - description: "desc".to_string(), - }, - ) - .unwrap(); - let error = builder - .add_flag_declaration( - Source::File("second.txt".to_string()), - FlagDeclaration { - name: "foo".to_string(), - namespace: "ns".to_string(), - description: "desc".to_string(), - }, - ) - .unwrap_err(); - assert_eq!( - &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(), - namespace: "ns".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!["bar".to_string(), "foo".to_string()] - ); - } - - #[test] - fn test_add_flag_value() { - let mut builder = CacheBuilder::new("com.example".to_string()).unwrap(); - let error = builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "com.example".to_string(), - name: "foo".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap_err(); - assert_eq!( - &format!("{:?}", error), - "failed to set values for flag com.example/foo from : flag not declared" - ); - - builder - .add_flag_declaration( - Source::File("first.txt".to_string()), - FlagDeclaration { - name: "foo".to_string(), - namespace: "ns".to_string(), - description: "desc".to_string(), - }, - ) - .unwrap(); - - builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "com.example".to_string(), - name: "foo".to_string(), - state: FlagState::Disabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap(); - - builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "com.example".to_string(), - name: "foo".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadWrite, - }, - ) - .unwrap(); - - // different package -> no-op - let error = builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "some_other_package".to_string(), - name: "foo".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap_err(); - assert_eq!(&format!("{:?}", error), "failed to set values for flag some_other_package/foo from : expected package com.example"); - - 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_package() { - CacheBuilder::new("".to_string()).unwrap_err(); - } - - #[test] - fn test_reject_empty_flag_declaration_fields() { - let mut builder = CacheBuilder::new("com.example".to_string()).unwrap(); - - let error = builder - .add_flag_declaration( - Source::Memory, - FlagDeclaration { - name: "".to_string(), - namespace: "ns".to_string(), - description: "Description".to_string(), - }, - ) - .unwrap_err(); - assert_eq!(&format!("{:?}", error), "bad flag name"); - - let error = builder - .add_flag_declaration( - Source::Memory, - FlagDeclaration { - name: "foo".to_string(), - namespace: "ns".to_string(), - description: "".to_string(), - }, - ) - .unwrap_err(); - assert_eq!(&format!("{:?}", error), "empty flag description"); - } - - #[test] - fn test_reject_empty_flag_value_files() { - let mut builder = CacheBuilder::new("com.example".to_string()).unwrap(); - builder - .add_flag_declaration( - Source::Memory, - FlagDeclaration { - name: "foo".to_string(), - namespace: "ns".to_string(), - description: "desc".to_string(), - }, - ) - .unwrap(); - - let error = builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "".to_string(), - name: "foo".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap_err(); - assert_eq!(&format!("{:?}", error), "bad flag package"); - - let error = builder - .add_flag_value( - Source::Memory, - FlagValue { - package: "com.example".to_string(), - name: "".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap_err(); - assert_eq!(&format!("{:?}", error), "bad 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)); - } -} diff --git a/tools/aconfig/src/codegen_cpp.rs b/tools/aconfig/src/codegen_cpp.rs index 37b058df4f..2944e8a2aa 100644 --- a/tools/aconfig/src/codegen_cpp.rs +++ b/tools/aconfig/src/codegen_cpp.rs @@ -18,15 +18,16 @@ 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; +use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag}; -pub fn generate_cpp_code(cache: &Cache) -> Result { - let package = cache.package(); +pub fn generate_cpp_code<'a, I>(package: &str, parsed_flags_iter: I) -> Result +where + I: Iterator, +{ let class_elements: Vec = - cache.iter().map(|item| create_class_element(package, item)).collect(); + parsed_flags_iter.map(|pf| create_class_element(package, pf)).collect(); let readwrite = class_elements.iter().any(|item| item.readwrite); let header = package.replace('.', "_"); let cpp_namespace = package.replace('.', "::"); @@ -63,162 +64,68 @@ struct ClassElement { pub device_config_flag: String, } -fn create_class_element(package: &str, item: &Item) -> ClassElement { +fn create_class_element(package: &str, pf: &ProtoParsedFlag) -> ClassElement { ClassElement { - readwrite: item.permission == Permission::ReadWrite, - default_value: if item.state == FlagState::Enabled { + readwrite: pf.permission() == ProtoFlagPermission::READ_WRITE, + default_value: if pf.state() == ProtoFlagState::ENABLED { "true".to_string() } else { "false".to_string() }, - flag_name: item.name.clone(), - device_config_namespace: item.namespace.to_string(), - device_config_flag: codegen::create_device_config_ident(package, &item.name) - .expect("values checked at cache creation time"), + flag_name: pf.name().to_string(), + device_config_namespace: pf.namespace().to_string(), + device_config_flag: codegen::create_device_config_ident(package, pf.name()) + .expect("values checked at flag parse time"), } } #[cfg(test)] 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 package = "com.example"; - let mut builder = CacheBuilder::new(package.to_string()).unwrap(); - builder - .add_flag_declaration( - Source::File("aconfig_one.txt".to_string()), - FlagDeclaration { - name: "my_flag_one".to_string(), - namespace: "ns".to_string(), - description: "buildtime disable".to_string(), - }, - ) - .unwrap() - .add_flag_value( - Source::Memory, - FlagValue { - package: package.to_string(), - name: "my_flag_one".to_string(), - state: FlagState::Disabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap() - .add_flag_declaration( - Source::File("aconfig_two.txt".to_string()), - FlagDeclaration { - name: "my_flag_two".to_string(), - namespace: "ns".to_string(), - description: "buildtime enable".to_string(), - }, - ) - .unwrap() - .add_flag_value( - Source::Memory, - FlagValue { - package: package.to_string(), - name: "my_flag_two".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadOnly, - }, - ) - .unwrap(); - let cache = builder.build(); - let expect_content = r#"#ifndef com_example_HEADER_H - #define com_example_HEADER_H + fn test_generate_cpp_code() { + let parsed_flags = crate::test::parse_test_flags(); + let generated = + generate_cpp_code(crate::test::TEST_PACKAGE, parsed_flags.parsed_flag.iter()).unwrap(); + assert_eq!("aconfig/com_android_aconfig_test.h", format!("{}", generated.path.display())); + let expected = r#" +#ifndef com_android_aconfig_test_HEADER_H +#define com_android_aconfig_test_HEADER_H +#include - namespace com::example { +using namespace server_configurable_flags; - static const bool my_flag_one() { - return false; - } - - static const bool my_flag_two() { - return true; - } - - } - #endif - "#; - let file = generate_cpp_code(&cache).unwrap(); - assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap()); - assert_eq!( - expect_content.replace(' ', ""), - String::from_utf8(file.contents).unwrap().replace(' ', "") - ); +namespace com::android::aconfig::test { + static const bool disabled_ro() { + return false; } - #[test] - fn test_cpp_codegen_runtime_flag() { - let package = "com.example"; - let mut builder = CacheBuilder::new(package.to_string()).unwrap(); - builder - .add_flag_declaration( - Source::File("aconfig_one.txt".to_string()), - FlagDeclaration { - name: "my_flag_one".to_string(), - namespace: "ns".to_string(), - description: "buildtime disable".to_string(), - }, - ) - .unwrap() - .add_flag_declaration( - Source::File("aconfig_two.txt".to_string()), - FlagDeclaration { - name: "my_flag_two".to_string(), - namespace: "ns".to_string(), - description: "runtime enable".to_string(), - }, - ) - .unwrap() - .add_flag_value( - Source::Memory, - FlagValue { - package: package.to_string(), - name: "my_flag_two".to_string(), - state: FlagState::Enabled, - permission: Permission::ReadWrite, - }, - ) - .unwrap(); - let cache = builder.build(); - let expect_content = r#"#ifndef com_example_HEADER_H - #define com_example_HEADER_H + static const bool disabled_rw() { + return GetServerConfigurableFlag( + "aconfig_test", + "com.android.aconfig.test.disabled_rw", + "false") == "true"; + } - #include - using namespace server_configurable_flags; + static const bool enabled_ro() { + return true; + } - namespace com::example { - - static const bool my_flag_one() { - return GetServerConfigurableFlag( - "ns", - "com.example.my_flag_one", - "false") == "true"; - } - - static const bool my_flag_two() { - return GetServerConfigurableFlag( - "ns", - "com.example.my_flag_two", - "true") == "true"; - } - - } - #endif - "#; - let file = generate_cpp_code(&cache).unwrap(); - assert_eq!("aconfig/com_example.h", file.path.to_str().unwrap()); + static const bool enabled_rw() { + return GetServerConfigurableFlag( + "aconfig_test", + "com.android.aconfig.test.enabled_rw", + "true") == "true"; + } +} +#endif +"#; assert_eq!( None, crate::test::first_significant_code_diff( - expect_content, - &String::from_utf8(file.contents).unwrap() + expected, + &String::from_utf8(generated.contents).unwrap() ) ); } diff --git a/tools/aconfig/src/codegen_java.rs b/tools/aconfig/src/codegen_java.rs index 4f82220809..0d1b281696 100644 --- a/tools/aconfig/src/codegen_java.rs +++ b/tools/aconfig/src/codegen_java.rs @@ -19,20 +19,18 @@ use serde::Serialize; use std::path::PathBuf; use tinytemplate::TinyTemplate; -use crate::aconfig::{FlagState, Permission}; -use crate::cache::{Cache, Item}; use crate::codegen; use crate::commands::OutputFile; +use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag}; -pub fn generate_java_code(cache: &Cache) -> Result> { - let package = cache.package(); +pub fn generate_java_code<'a, I>(package: &str, parsed_flags_iter: I) -> Result> +where + I: Iterator, +{ let class_elements: Vec = - cache.iter().map(|item| create_class_element(package, item)).collect(); - let is_read_write = class_elements.iter().any(|item| item.is_read_write); + parsed_flags_iter.map(|pf| create_class_element(package, pf)).collect(); + let is_read_write = class_elements.iter().any(|elem| elem.is_read_write); let context = Context { package_name: package.to_string(), is_read_write, class_elements }; - - let java_files = vec!["Flags.java", "FeatureFlagsImpl.java", "FeatureFlags.java"]; - let mut template = TinyTemplate::new(); template.add_template("Flags.java", include_str!("../templates/Flags.java.template"))?; template.add_template( @@ -45,7 +43,7 @@ pub fn generate_java_code(cache: &Cache) -> Result> { )?; let path: PathBuf = package.split('.').collect(); - java_files + ["Flags.java", "FeatureFlagsImpl.java", "FeatureFlags.java"] .iter() .map(|file| { Ok(OutputFile { @@ -68,25 +66,23 @@ struct ClassElement { pub default_value: String, pub device_config_namespace: String, pub device_config_flag: String, - pub flag_name_constant_suffix: String, pub is_read_write: bool, pub method_name: String, } -fn create_class_element(package: &str, item: &Item) -> ClassElement { - let device_config_flag = codegen::create_device_config_ident(package, &item.name) - .expect("values checked at cache creation time"); +fn create_class_element(package: &str, pf: &ProtoParsedFlag) -> ClassElement { + let device_config_flag = codegen::create_device_config_ident(package, pf.name()) + .expect("values checked at flag parse time"); ClassElement { - default_value: if item.state == FlagState::Enabled { + default_value: if pf.state() == ProtoFlagState::ENABLED { "true".to_string() } else { "false".to_string() }, - device_config_namespace: item.namespace.clone(), + device_config_namespace: pf.namespace().to_string(), device_config_flag, - flag_name_constant_suffix: item.name.to_ascii_uppercase(), - is_read_write: item.permission == Permission::ReadWrite, - method_name: format_java_method_name(&item.name), + is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE, + method_name: format_java_method_name(pf.name()), } } @@ -113,8 +109,9 @@ mod tests { #[test] fn test_generate_java_code() { - let cache = crate::test::create_cache(); - let generated_files = generate_java_code(&cache).unwrap(); + let parsed_flags = crate::test::parse_test_flags(); + let generated_files = + generate_java_code(crate::test::TEST_PACKAGE, parsed_flags.parsed_flag.iter()).unwrap(); let expect_flags_content = r#" package com.android.aconfig.test; public final class Flags { @@ -131,7 +128,6 @@ mod tests { return FEATURE_FLAGS.enabledRw(); } private static FeatureFlags FEATURE_FLAGS = new FeatureFlagsImpl(); - } "#; let expected_featureflagsimpl_content = r#" diff --git a/tools/aconfig/src/codegen_rust.rs b/tools/aconfig/src/codegen_rust.rs index 98caeae5cb..f931418a7b 100644 --- a/tools/aconfig/src/codegen_rust.rs +++ b/tools/aconfig/src/codegen_rust.rs @@ -18,18 +18,19 @@ use anyhow::Result; use serde::Serialize; use tinytemplate::TinyTemplate; -use crate::aconfig::{FlagState, Permission}; -use crate::cache::{Cache, Item}; use crate::codegen; use crate::commands::OutputFile; +use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag}; -pub fn generate_rust_code(cache: &Cache) -> Result { - let package = cache.package(); - let parsed_flags: Vec = - cache.iter().map(|item| TemplateParsedFlag::new(package, item)).collect(); +pub fn generate_rust_code<'a, I>(package: &str, parsed_flags_iter: I) -> Result +where + I: Iterator, +{ + let template_flags: Vec = + parsed_flags_iter.map(|pf| TemplateParsedFlag::new(package, pf)).collect(); let context = TemplateContext { package: package.to_string(), - parsed_flags, + template_flags, modules: package.split('.').map(|s| s.to_string()).collect::>(), }; let mut template = TinyTemplate::new(); @@ -42,7 +43,7 @@ pub fn generate_rust_code(cache: &Cache) -> Result { #[derive(Serialize)] struct TemplateContext { pub package: String, - pub parsed_flags: Vec, + pub template_flags: Vec, pub modules: Vec, } @@ -61,17 +62,17 @@ struct TemplateParsedFlag { impl TemplateParsedFlag { #[allow(clippy::nonminimal_bool)] - fn new(package: &str, item: &Item) -> Self { + fn new(package: &str, pf: &ProtoParsedFlag) -> Self { let template = TemplateParsedFlag { - name: item.name.clone(), - device_config_namespace: item.namespace.to_string(), - device_config_flag: codegen::create_device_config_ident(package, &item.name) - .expect("values checked at cache creation time"), - is_read_only_enabled: item.permission == Permission::ReadOnly - && item.state == FlagState::Enabled, - is_read_only_disabled: item.permission == Permission::ReadOnly - && item.state == FlagState::Disabled, - is_read_write: item.permission == Permission::ReadWrite, + name: pf.name().to_string(), + device_config_namespace: pf.namespace().to_string(), + device_config_flag: codegen::create_device_config_ident(package, pf.name()) + .expect("values checked at flag parse time"), + is_read_only_enabled: pf.permission() == ProtoFlagPermission::READ_ONLY + && pf.state() == ProtoFlagState::ENABLED, + is_read_only_disabled: pf.permission() == ProtoFlagPermission::READ_ONLY + && pf.state() == ProtoFlagState::DISABLED, + is_read_write: pf.permission() == ProtoFlagPermission::READ_WRITE, }; #[rustfmt::skip] debug_assert!( @@ -93,8 +94,9 @@ mod tests { #[test] fn test_generate_rust_code() { - let cache = crate::test::create_cache(); - let generated = generate_rust_code(&cache).unwrap(); + let parsed_flags = crate::test::parse_test_flags(); + let generated = + generate_rust_code(crate::test::TEST_PACKAGE, parsed_flags.parsed_flag.iter()).unwrap(); assert_eq!("src/lib.rs", format!("{}", generated.path.display())); let expected = r#" pub mod com { diff --git a/tools/aconfig/src/commands.rs b/tools/aconfig/src/commands.rs index eb860b0bac..f295697db7 100644 --- a/tools/aconfig/src/commands.rs +++ b/tools/aconfig/src/commands.rs @@ -14,105 +14,160 @@ * limitations under the License. */ -use anyhow::{ensure, Context, Result}; +use anyhow::{bail, ensure, Context, Result}; use clap::ValueEnum; use protobuf::Message; -use serde::{Deserialize, Serialize}; -use std::fmt; use std::io::Read; use std::path::PathBuf; -use crate::aconfig::{FlagDeclarations, FlagState, FlagValue, Permission}; -use crate::cache::{Cache, CacheBuilder, Item}; use crate::codegen_cpp::generate_cpp_code; use crate::codegen_java::generate_java_code; use crate::codegen_rust::generate_rust_code; -use crate::protos::ProtoParsedFlags; - -#[derive(Serialize, Deserialize, Clone, Debug)] -pub enum Source { - #[allow(dead_code)] // only used in unit tests - Memory, - File(String), -} - -impl fmt::Display for Source { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - match self { - Self::Memory => write!(f, ""), - Self::File(path) => write!(f, "{}", path), - } - } -} +use crate::protos::{ + ProtoFlagPermission, ProtoFlagState, ProtoParsedFlag, ProtoParsedFlags, ProtoTracepoint, +}; pub struct Input { - pub source: Source, + pub source: String, pub reader: Box, } +impl Input { + fn try_parse_flags(&mut self) -> Result { + let mut buffer = Vec::new(); + self.reader.read_to_end(&mut buffer)?; + crate::protos::parsed_flags::try_from_binary_proto(&buffer) + } +} + pub struct OutputFile { pub path: PathBuf, // relative to some root directory only main knows about pub contents: Vec, } -pub fn create_cache(package: &str, declarations: Vec, values: Vec) -> Result { - let mut builder = CacheBuilder::new(package.to_owned())?; +const DEFAULT_FLAG_STATE: ProtoFlagState = ProtoFlagState::DISABLED; +const DEFAULT_FLAG_PERMISSION: ProtoFlagPermission = ProtoFlagPermission::READ_WRITE; + +pub fn parse_flags(package: &str, declarations: Vec, values: Vec) -> Result> { + let mut parsed_flags = ProtoParsedFlags::new(); for mut input in declarations { let mut contents = String::new(); input.reader.read_to_string(&mut contents)?; - let dec_list = FlagDeclarations::try_from_text_proto(&contents) + + let flag_declarations = crate::protos::flag_declarations::try_from_text_proto(&contents) .with_context(|| format!("Failed to parse {}", input.source))?; ensure!( - package == dec_list.package, + package == flag_declarations.package(), "Failed to parse {}: expected package {}, got {}", input.source, package, - dec_list.package + flag_declarations.package() ); - for d in dec_list.flags.into_iter() { - builder.add_flag_declaration(input.source.clone(), d)?; + for mut flag_declaration in flag_declarations.flag.into_iter() { + crate::protos::flag_declaration::verify_fields(&flag_declaration) + .with_context(|| format!("Failed to parse {}", input.source))?; + + // create ParsedFlag using FlagDeclaration and default values + let mut parsed_flag = ProtoParsedFlag::new(); + parsed_flag.set_package(package.to_string()); + parsed_flag.set_name(flag_declaration.take_name()); + parsed_flag.set_namespace(flag_declaration.take_namespace()); + parsed_flag.set_description(flag_declaration.take_description()); + parsed_flag.set_state(DEFAULT_FLAG_STATE); + parsed_flag.set_permission(DEFAULT_FLAG_PERMISSION); + let mut tracepoint = ProtoTracepoint::new(); + tracepoint.set_source(input.source.clone()); + tracepoint.set_state(DEFAULT_FLAG_STATE); + tracepoint.set_permission(DEFAULT_FLAG_PERMISSION); + parsed_flag.trace.push(tracepoint); + + // verify ParsedFlag looks reasonable + crate::protos::parsed_flag::verify_fields(&parsed_flag)?; + + // verify ParsedFlag can be added + ensure!( + parsed_flags.parsed_flag.iter().all(|other| other.name() != parsed_flag.name()), + "failed to declare flag {} from {}: flag already declared", + parsed_flag.name(), + input.source + ); + + // add ParsedFlag to ParsedFlags + parsed_flags.parsed_flag.push(parsed_flag); } } for mut input in values { let mut contents = String::new(); input.reader.read_to_string(&mut contents)?; - let values_list = FlagValue::try_from_text_proto_list(&contents) + let flag_values = crate::protos::flag_values::try_from_text_proto(&contents) .with_context(|| format!("Failed to parse {}", input.source))?; - for v in values_list { - // TODO: warn about flag values that do not take effect? - let _ = builder.add_flag_value(input.source.clone(), v); + for flag_value in flag_values.flag_value.into_iter() { + crate::protos::flag_value::verify_fields(&flag_value) + .with_context(|| format!("Failed to parse {}", input.source))?; + + let Some(parsed_flag) = parsed_flags.parsed_flag.iter_mut().find(|pf| pf.package() == flag_value.package() && pf.name() == flag_value.name()) else { + // (silently) skip unknown flags + continue; + }; + + parsed_flag.set_state(flag_value.state()); + parsed_flag.set_permission(flag_value.permission()); + let mut tracepoint = ProtoTracepoint::new(); + tracepoint.set_source(input.source.clone()); + tracepoint.set_state(flag_value.state()); + tracepoint.set_permission(flag_value.permission()); + parsed_flag.trace.push(tracepoint); } } - Ok(builder.build()) -} - -pub fn create_java_lib(cache: Cache) -> Result> { - generate_java_code(&cache) -} - -pub fn create_cpp_lib(cache: Cache) -> Result { - generate_cpp_code(&cache) -} - -pub fn create_rust_lib(cache: Cache) -> Result { - generate_rust_code(&cache) -} - -pub fn create_device_config_defaults(caches: Vec) -> Result> { + crate::protos::parsed_flags::verify_fields(&parsed_flags)?; let mut output = Vec::new(); - for item in sort_and_iter_items(caches).filter(|item| item.permission == Permission::ReadWrite) + parsed_flags.write_to_vec(&mut output)?; + Ok(output) +} + +pub fn create_java_lib(mut input: Input) -> Result> { + let parsed_flags = input.try_parse_flags()?; + let Some(package) = find_unique_package(&parsed_flags) else { + bail!("no parsed flags, or the parsed flags use different packages"); + }; + generate_java_code(package, parsed_flags.parsed_flag.iter()) +} + +pub fn create_cpp_lib(mut input: Input) -> Result { + let parsed_flags = input.try_parse_flags()?; + let Some(package) = find_unique_package(&parsed_flags) else { + bail!("no parsed flags, or the parsed flags use different packages"); + }; + generate_cpp_code(package, parsed_flags.parsed_flag.iter()) +} + +pub fn create_rust_lib(mut input: Input) -> Result { + let parsed_flags = input.try_parse_flags()?; + let Some(package) = find_unique_package(&parsed_flags) else { + bail!("no parsed flags, or the parsed flags use different packages"); + }; + generate_rust_code(package, parsed_flags.parsed_flag.iter()) +} + +pub fn create_device_config_defaults(mut input: Input) -> Result> { + let parsed_flags = input.try_parse_flags()?; + let mut output = Vec::new(); + for parsed_flag in parsed_flags + .parsed_flag + .into_iter() + .filter(|pf| pf.permission() == ProtoFlagPermission::READ_WRITE) { let line = format!( "{}:{}.{}={}\n", - item.namespace, - item.package, - item.name, - match item.state { - FlagState::Enabled => "enabled", - FlagState::Disabled => "disabled", + parsed_flag.namespace(), + parsed_flag.package(), + parsed_flag.name(), + match parsed_flag.state() { + ProtoFlagState::ENABLED => "enabled", + ProtoFlagState::DISABLED => "disabled", } ); output.extend_from_slice(line.as_bytes()); @@ -120,17 +175,21 @@ pub fn create_device_config_defaults(caches: Vec) -> Result> { Ok(output) } -pub fn create_device_config_sysprops(caches: Vec) -> Result> { +pub fn create_device_config_sysprops(mut input: Input) -> Result> { + let parsed_flags = input.try_parse_flags()?; let mut output = Vec::new(); - for item in sort_and_iter_items(caches).filter(|item| item.permission == Permission::ReadWrite) + for parsed_flag in parsed_flags + .parsed_flag + .into_iter() + .filter(|pf| pf.permission() == ProtoFlagPermission::READ_WRITE) { let line = format!( "persist.device_config.{}.{}={}\n", - item.package, - item.name, - match item.state { - FlagState::Enabled => "true", - FlagState::Disabled => "false", + parsed_flag.package(), + parsed_flag.name(), + match parsed_flag.state() { + ProtoFlagState::ENABLED => "true", + ProtoFlagState::DISABLED => "false", } ); output.extend_from_slice(line.as_bytes()); @@ -145,177 +204,118 @@ pub enum DumpFormat { Protobuf, } -pub fn dump_cache(caches: Vec, format: DumpFormat) -> Result> { +pub fn dump_parsed_flags(mut input: Vec, format: DumpFormat) -> Result> { + let individually_parsed_flags: Result> = + input.iter_mut().map(|i| i.try_parse_flags()).collect(); + let parsed_flags: ProtoParsedFlags = + crate::protos::parsed_flags::merge(individually_parsed_flags?)?; + let mut output = Vec::new(); match format { DumpFormat::Text => { - for item in sort_and_iter_items(caches) { + for parsed_flag in parsed_flags.parsed_flag.into_iter() { let line = format!( "{}/{}: {:?} {:?}\n", - item.package, item.name, item.state, item.permission + parsed_flag.package(), + parsed_flag.name(), + parsed_flag.state(), + parsed_flag.permission() ); output.extend_from_slice(line.as_bytes()); } } DumpFormat::Debug => { - for item in sort_and_iter_items(caches) { - let line = format!("{:#?}\n", item); + for parsed_flag in parsed_flags.parsed_flag.into_iter() { + let line = format!("{:#?}\n", parsed_flag); output.extend_from_slice(line.as_bytes()); } } DumpFormat::Protobuf => { - for cache in sort_and_iter_caches(caches) { - let parsed_flags: ProtoParsedFlags = cache.into(); - parsed_flags.write_to_vec(&mut output)?; - } + parsed_flags.write_to_vec(&mut output)?; } } Ok(output) } -fn sort_and_iter_items(caches: Vec) -> impl Iterator { - sort_and_iter_caches(caches).flat_map(|cache| cache.into_iter()) -} - -fn sort_and_iter_caches(mut caches: Vec) -> impl Iterator { - caches.sort_by_cached_key(|cache| cache.package().to_string()); - caches.into_iter() +fn find_unique_package(parsed_flags: &ProtoParsedFlags) -> Option<&str> { + let Some(package) = parsed_flags.parsed_flag.first().map(|pf| pf.package()) else { + return None; + }; + if parsed_flags.parsed_flag.iter().any(|pf| pf.package() != package) { + return None; + } + Some(package) } #[cfg(test)] mod tests { use super::*; - use crate::aconfig::{FlagState, Permission}; - - fn create_test_cache_com_example() -> Cache { - let s = r#" - package: "com.example" - flag { - name: "a" - namespace: "ns" - description: "Description of a" - } - flag { - name: "b" - namespace: "ns" - description: "Description of b" - } - "#; - let declarations = vec![Input { source: Source::Memory, reader: Box::new(s.as_bytes()) }]; - let o = r#" - flag_value { - package: "com.example" - name: "a" - state: DISABLED - permission: READ_ONLY - } - "#; - let values = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }]; - create_cache("com.example", declarations, values).unwrap() - } - - fn create_test_cache_com_other() -> Cache { - let s = r#" - package: "com.other" - flag { - name: "c" - namespace: "ns" - description: "Description of c" - } - "#; - let declarations = vec![Input { source: Source::Memory, reader: Box::new(s.as_bytes()) }]; - let o = r#" - flag_value { - package: "com.other" - name: "c" - state: DISABLED - permission: READ_ONLY - } - "#; - let values = vec![Input { source: Source::Memory, reader: Box::new(o.as_bytes()) }]; - create_cache("com.other", declarations, values).unwrap() - } #[test] - fn test_create_cache() { - let caches = create_test_cache_com_example(); // calls create_cache - let item = caches.iter().find(|&item| item.name == "a").unwrap(); - assert_eq!(FlagState::Disabled, item.state); - assert_eq!(Permission::ReadOnly, item.permission); + fn test_parse_flags() { + let parsed_flags = crate::test::parse_test_flags(); // calls parse_flags + crate::protos::parsed_flags::verify_fields(&parsed_flags).unwrap(); + + let enabled_ro = + parsed_flags.parsed_flag.iter().find(|pf| pf.name() == "enabled_ro").unwrap(); + assert!(crate::protos::parsed_flag::verify_fields(enabled_ro).is_ok()); + assert_eq!("com.android.aconfig.test", enabled_ro.package()); + assert_eq!("enabled_ro", enabled_ro.name()); + assert_eq!("This flag is ENABLED + READ_ONLY", enabled_ro.description()); + assert_eq!(ProtoFlagState::ENABLED, enabled_ro.state()); + assert_eq!(ProtoFlagPermission::READ_ONLY, enabled_ro.permission()); + assert_eq!(3, enabled_ro.trace.len()); + assert_eq!("tests/test.aconfig", enabled_ro.trace[0].source()); + assert_eq!(ProtoFlagState::DISABLED, enabled_ro.trace[0].state()); + assert_eq!(ProtoFlagPermission::READ_WRITE, enabled_ro.trace[0].permission()); + assert_eq!("tests/first.values", enabled_ro.trace[1].source()); + assert_eq!(ProtoFlagState::DISABLED, enabled_ro.trace[1].state()); + assert_eq!(ProtoFlagPermission::READ_WRITE, enabled_ro.trace[1].permission()); + assert_eq!("tests/second.values", enabled_ro.trace[2].source()); + assert_eq!(ProtoFlagState::ENABLED, enabled_ro.trace[2].state()); + assert_eq!(ProtoFlagPermission::READ_ONLY, enabled_ro.trace[2].permission()); + + assert_eq!(4, parsed_flags.parsed_flag.len()); + for pf in parsed_flags.parsed_flag.iter() { + let first = pf.trace.first().unwrap(); + assert_eq!(DEFAULT_FLAG_STATE, first.state()); + assert_eq!(DEFAULT_FLAG_PERMISSION, first.permission()); + + let last = pf.trace.last().unwrap(); + assert_eq!(pf.state(), last.state()); + assert_eq!(pf.permission(), last.permission()); + } } #[test] fn test_create_device_config_defaults() { - let caches = vec![crate::test::create_cache()]; - let bytes = create_device_config_defaults(caches).unwrap(); + let input = parse_test_flags_as_input(); + let bytes = create_device_config_defaults(input).unwrap(); let text = std::str::from_utf8(&bytes).unwrap(); assert_eq!("aconfig_test:com.android.aconfig.test.disabled_rw=disabled\naconfig_test:com.android.aconfig.test.enabled_rw=enabled\n", text); } #[test] fn test_create_device_config_sysprops() { - let caches = vec![crate::test::create_cache()]; - let bytes = create_device_config_sysprops(caches).unwrap(); + let input = parse_test_flags_as_input(); + let bytes = create_device_config_sysprops(input).unwrap(); let text = std::str::from_utf8(&bytes).unwrap(); assert_eq!("persist.device_config.com.android.aconfig.test.disabled_rw=false\npersist.device_config.com.android.aconfig.test.enabled_rw=true\n", text); } #[test] fn test_dump_text_format() { - let caches = vec![create_test_cache_com_example()]; - let bytes = dump_cache(caches, DumpFormat::Text).unwrap(); + let input = parse_test_flags_as_input(); + let bytes = dump_parsed_flags(vec![input], DumpFormat::Text).unwrap(); let text = std::str::from_utf8(&bytes).unwrap(); - assert!(text.contains("a: Disabled")); + assert!(text.contains("com.android.aconfig.test/disabled_ro: DISABLED READ_ONLY")); } - #[test] - fn test_dump_protobuf_format() { - use crate::protos::{ProtoFlagPermission, ProtoFlagState, ProtoTracepoint}; - use protobuf::Message; - - let caches = vec![create_test_cache_com_example()]; - let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap(); - let actual = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap(); - - assert_eq!( - vec!["a".to_string(), "b".to_string()], - actual.parsed_flag.iter().map(|item| item.name.clone().unwrap()).collect::>() - ); - - let item = - actual.parsed_flag.iter().find(|item| item.name == Some("b".to_string())).unwrap(); - assert_eq!(item.package(), "com.example"); - assert_eq!(item.name(), "b"); - assert_eq!(item.description(), "Description of b"); - assert_eq!(item.state(), ProtoFlagState::DISABLED); - assert_eq!(item.permission(), ProtoFlagPermission::READ_WRITE); - let mut tp = ProtoTracepoint::new(); - tp.set_source("".to_string()); - tp.set_state(ProtoFlagState::DISABLED); - tp.set_permission(ProtoFlagPermission::READ_WRITE); - assert_eq!(item.trace, vec![tp]); - } - - #[test] - fn test_dump_multiple_caches() { - let caches = vec![create_test_cache_com_example(), create_test_cache_com_other()]; - let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap(); - let dump = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap(); - assert_eq!( - dump.parsed_flag - .iter() - .map(|parsed_flag| format!("{}/{}", parsed_flag.package(), parsed_flag.name())) - .collect::>(), - vec![ - "com.example/a".to_string(), - "com.example/b".to_string(), - "com.other/c".to_string() - ] - ); - - let caches = vec![create_test_cache_com_other(), create_test_cache_com_example()]; - let bytes = dump_cache(caches, DumpFormat::Protobuf).unwrap(); - let dump_reversed_input = ProtoParsedFlags::parse_from_bytes(&bytes).unwrap(); - assert_eq!(dump, dump_reversed_input); + fn parse_test_flags_as_input() -> Input { + let parsed_flags = crate::test::parse_test_flags(); + let binary_proto = parsed_flags.write_to_bytes().unwrap(); + let cursor = std::io::Cursor::new(binary_proto); + let reader = Box::new(cursor); + Input { source: "test.data".to_string(), reader } } } diff --git a/tools/aconfig/src/main.rs b/tools/aconfig/src/main.rs index 3a9a573f7b..e6a325dea7 100644 --- a/tools/aconfig/src/main.rs +++ b/tools/aconfig/src/main.rs @@ -16,7 +16,7 @@ //! `aconfig` is a build time tool to manage build time configurations, such as feature flags. -use anyhow::{anyhow, ensure, Result}; +use anyhow::{anyhow, bail, ensure, Result}; use clap::{builder::ArgAction, builder::EnumValueParser, Arg, ArgMatches, Command}; use core::any::Any; use std::fs; @@ -24,8 +24,6 @@ use std::io; use std::io::Write; use std::path::{Path, PathBuf}; -mod aconfig; -mod cache; mod codegen; mod codegen_cpp; mod codegen_java; @@ -36,8 +34,7 @@ mod protos; #[cfg(test)] mod test; -use crate::cache::Cache; -use commands::{DumpFormat, Input, OutputFile, Source}; +use commands::{DumpFormat, Input, OutputFile}; fn cli() -> Command { Command::new("aconfig") @@ -100,11 +97,19 @@ fn open_zero_or_more_files(matches: &ArgMatches, arg_name: &str) -> Result(arg_name).unwrap_or_default() { let file = Box::new(fs::File::open(path)?); - opened_files.push(Input { source: Source::File(path.to_string()), reader: file }); + opened_files.push(Input { source: path.to_string(), reader: file }); } Ok(opened_files) } +fn open_single_file(matches: &ArgMatches, arg_name: &str) -> Result { + let Some(path) = matches.get_one::(arg_name) else { + bail!("missing argument {}", arg_name); + }; + let file = Box::new(fs::File::open(path)?); + Ok(Input { source: path.to_string(), reader: file }) +} + fn write_output_file_realtive_to_dir(root: &Path, output_file: &OutputFile) -> Result<()> { ensure!( root.is_dir(), @@ -137,68 +142,46 @@ fn main() -> Result<()> { let package = get_required_arg::(sub_matches, "package")?; 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(package, declarations, values)?; + let output = commands::parse_flags(package, declarations, values)?; let path = get_required_arg::(sub_matches, "cache")?; - let file = fs::File::create(path)?; - cache.write_to_writer(file)?; + write_output_to_file_or_stdout(path, &output)?; } Some(("create-java-lib", sub_matches)) => { - let path = get_required_arg::(sub_matches, "cache")?; - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); + let cache = open_single_file(sub_matches, "cache")?; let generated_files = commands::create_java_lib(cache)?; + let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); generated_files .iter() .try_for_each(|file| write_output_file_realtive_to_dir(&dir, file))?; } Some(("create-cpp-lib", sub_matches)) => { - let path = get_required_arg::(sub_matches, "cache")?; - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); + let cache = open_single_file(sub_matches, "cache")?; let generated_file = commands::create_cpp_lib(cache)?; + let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); write_output_file_realtive_to_dir(&dir, &generated_file)?; } Some(("create-rust-lib", sub_matches)) => { - let path = get_required_arg::(sub_matches, "cache")?; - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); + let cache = open_single_file(sub_matches, "cache")?; let generated_file = commands::create_rust_lib(cache)?; + let dir = PathBuf::from(get_required_arg::(sub_matches, "out")?); write_output_file_realtive_to_dir(&dir, &generated_file)?; } Some(("create-device-config-defaults", sub_matches)) => { - let mut caches = Vec::new(); - for path in sub_matches.get_many::("cache").unwrap_or_default() { - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - caches.push(cache); - } - let output = commands::create_device_config_defaults(caches)?; + let cache = open_single_file(sub_matches, "cache")?; + let output = commands::create_device_config_defaults(cache)?; let path = get_required_arg::(sub_matches, "out")?; write_output_to_file_or_stdout(path, &output)?; } Some(("create-device-config-sysprops", sub_matches)) => { - let mut caches = Vec::new(); - for path in sub_matches.get_many::("cache").unwrap_or_default() { - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - caches.push(cache); - } - let output = commands::create_device_config_sysprops(caches)?; + let cache = open_single_file(sub_matches, "cache")?; + let output = commands::create_device_config_sysprops(cache)?; let path = get_required_arg::(sub_matches, "out")?; write_output_to_file_or_stdout(path, &output)?; } Some(("dump", sub_matches)) => { - let mut caches = Vec::new(); - for path in sub_matches.get_many::("cache").unwrap_or_default() { - let file = fs::File::open(path)?; - let cache = Cache::read_from_reader(file)?; - caches.push(cache); - } + let input = open_zero_or_more_files(sub_matches, "cache")?; let format = get_required_arg::(sub_matches, "format")?; - let output = commands::dump_cache(caches, *format)?; + let output = commands::dump_parsed_flags(input, *format)?; let path = get_required_arg::(sub_matches, "out")?; write_output_to_file_or_stdout(path, &output)?; } diff --git a/tools/aconfig/src/protos.rs b/tools/aconfig/src/protos.rs index 604fd35984..fb5dab4fe2 100644 --- a/tools/aconfig/src/protos.rs +++ b/tools/aconfig/src/protos.rs @@ -63,10 +63,622 @@ pub use auto_generated::*; use anyhow::Result; -pub fn try_from_text_proto(s: &str) -> Result +fn try_from_text_proto(s: &str) -> Result where T: protobuf::MessageFull, { // warning: parse_from_str does not check if required fields are set protobuf::text_format::parse_from_str(s).map_err(|e| e.into()) } + +pub mod flag_declaration { + use super::*; + use crate::codegen; + use anyhow::ensure; + + pub fn verify_fields(pdf: &ProtoFlagDeclaration) -> Result<()> { + ensure!(codegen::is_valid_name_ident(pdf.name()), "bad flag declaration: bad name"); + ensure!(codegen::is_valid_name_ident(pdf.namespace()), "bad flag declaration: bad name"); + ensure!(!pdf.description().is_empty(), "bad flag declaration: empty description"); + Ok(()) + } +} + +pub mod flag_declarations { + use super::*; + use crate::codegen; + use anyhow::ensure; + + pub fn try_from_text_proto(s: &str) -> Result { + let pdf: ProtoFlagDeclarations = super::try_from_text_proto(s)?; + verify_fields(&pdf)?; + Ok(pdf) + } + + pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> { + ensure!( + codegen::is_valid_package_ident(pdf.package()), + "bad flag declarations: bad package" + ); + for flag_declaration in pdf.flag.iter() { + super::flag_declaration::verify_fields(flag_declaration)?; + } + Ok(()) + } +} + +pub mod flag_value { + use super::*; + use crate::codegen; + use anyhow::ensure; + + pub fn verify_fields(fv: &ProtoFlagValue) -> Result<()> { + ensure!(codegen::is_valid_package_ident(fv.package()), "bad flag value: bad package"); + ensure!(codegen::is_valid_name_ident(fv.name()), "bad flag value: bad name"); + Ok(()) + } +} + +pub mod flag_values { + use super::*; + + pub fn try_from_text_proto(s: &str) -> Result { + let pfv: ProtoFlagValues = super::try_from_text_proto(s)?; + verify_fields(&pfv)?; + Ok(pfv) + } + + pub fn verify_fields(pfv: &ProtoFlagValues) -> Result<()> { + for flag_value in pfv.flag_value.iter() { + super::flag_value::verify_fields(flag_value)?; + } + Ok(()) + } +} + +pub mod tracepoint { + use super::*; + use anyhow::ensure; + + pub fn verify_fields(tp: &ProtoTracepoint) -> Result<()> { + ensure!(!tp.source().is_empty(), "bad tracepoint: empty source"); + Ok(()) + } +} + +pub mod parsed_flag { + use super::*; + use crate::codegen; + use anyhow::ensure; + + pub fn verify_fields(pf: &ProtoParsedFlag) -> Result<()> { + ensure!(codegen::is_valid_package_ident(pf.package()), "bad parsed flag: bad package"); + ensure!(codegen::is_valid_name_ident(pf.name()), "bad parsed flag: bad name"); + ensure!(codegen::is_valid_name_ident(pf.namespace()), "bad parsed flag: bad namespace"); + ensure!(!pf.description().is_empty(), "bad parsed flag: empty description"); + ensure!(!pf.trace.is_empty(), "bad parsed flag: empty trace"); + for tp in pf.trace.iter() { + super::tracepoint::verify_fields(tp)?; + } + Ok(()) + } +} + +pub mod parsed_flags { + use super::*; + use anyhow::bail; + use std::cmp::Ordering; + + pub fn try_from_binary_proto(bytes: &[u8]) -> Result { + let message: ProtoParsedFlags = protobuf::Message::parse_from_bytes(bytes)?; + verify_fields(&message)?; + Ok(message) + } + + pub fn verify_fields(pf: &ProtoParsedFlags) -> Result<()> { + let mut previous: Option<&ProtoParsedFlag> = None; + for parsed_flag in pf.parsed_flag.iter() { + if let Some(prev) = previous { + let a = create_sorting_key(prev); + let b = create_sorting_key(parsed_flag); + match a.cmp(&b) { + Ordering::Less => {} + Ordering::Equal => bail!("bad parsed flags: duplicate flag {}", a), + Ordering::Greater => { + bail!("bad parsed flags: not sorted: {} comes before {}", a, b) + } + } + } + super::parsed_flag::verify_fields(parsed_flag)?; + previous = Some(parsed_flag); + } + Ok(()) + } + + pub fn merge(parsed_flags: Vec) -> Result { + let mut merged = ProtoParsedFlags::new(); + for mut pfs in parsed_flags.into_iter() { + merged.parsed_flag.append(&mut pfs.parsed_flag); + } + merged.parsed_flag.sort_by_cached_key(create_sorting_key); + verify_fields(&merged)?; + Ok(merged) + } + + fn create_sorting_key(pf: &ProtoParsedFlag) -> String { + format!("{}.{}", pf.package(), pf.name()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_flag_declarations_try_from_text_proto() { + // valid input + let flag_declarations = flag_declarations::try_from_text_proto( + r#" +package: "com.foo.bar" +flag { + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." +} +flag { + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." +} +"#, + ) + .unwrap(); + assert_eq!(flag_declarations.package(), "com.foo.bar"); + let first = flag_declarations.flag.iter().find(|pf| pf.name() == "first").unwrap(); + assert_eq!(first.name(), "first"); + assert_eq!(first.namespace(), "first_ns"); + assert_eq!(first.description(), "This is the description of the first flag."); + let second = flag_declarations.flag.iter().find(|pf| pf.name() == "second").unwrap(); + assert_eq!(second.name(), "second"); + assert_eq!(second.namespace(), "second_ns"); + assert_eq!(second.description(), "This is the description of the second flag."); + + // bad input: missing package in flag declarations + let error = flag_declarations::try_from_text_proto( + r#" +flag { + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." +} +flag { + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("Message not initialized")); + + // bad input: missing namespace in flag declaration + let error = flag_declarations::try_from_text_proto( + r#" +package: "com.foo.bar" +flag { + name: "first" + description: "This is the description of the first flag." +} +flag { + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("Message not initialized")); + + // bad input: bad package name in flag declarations + let error = flag_declarations::try_from_text_proto( + r#" +package: "_com.FOO__BAR" +flag { + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." +} +flag { + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("bad flag declarations: bad package")); + + // bad input: bad name in flag declaration + let error = flag_declarations::try_from_text_proto( + r#" +package: "com.foo.bar" +flag { + name: "FIRST" + namespace: "first_ns" + description: "This is the description of the first flag." +} +flag { + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("bad flag declaration: bad name")); + } + + #[test] + fn test_flag_values_try_from_text_proto() { + // valid input + let flag_values = flag_values::try_from_text_proto( + r#" +flag_value { + package: "com.first" + name: "first" + state: DISABLED + permission: READ_ONLY +} +flag_value { + package: "com.second" + name: "second" + state: ENABLED + permission: READ_WRITE +} +"#, + ) + .unwrap(); + let first = flag_values.flag_value.iter().find(|fv| fv.name() == "first").unwrap(); + assert_eq!(first.package(), "com.first"); + assert_eq!(first.name(), "first"); + assert_eq!(first.state(), ProtoFlagState::DISABLED); + assert_eq!(first.permission(), ProtoFlagPermission::READ_ONLY); + let second = flag_values.flag_value.iter().find(|fv| fv.name() == "second").unwrap(); + assert_eq!(second.package(), "com.second"); + assert_eq!(second.name(), "second"); + assert_eq!(second.state(), ProtoFlagState::ENABLED); + assert_eq!(second.permission(), ProtoFlagPermission::READ_WRITE); + + // bad input: bad package in flag value + let error = flag_values::try_from_text_proto( + r#" +flag_value { + package: "COM.FIRST" + name: "first" + state: DISABLED + permission: READ_ONLY +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("bad flag value: bad package")); + + // bad input: bad name in flag value + let error = flag_values::try_from_text_proto( + r#" +flag_value { + package: "com.first" + name: "FIRST" + state: DISABLED + permission: READ_ONLY +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("bad flag value: bad name")); + + // bad input: missing state in flag value + let error = flag_values::try_from_text_proto( + r#" +flag_value { + package: "com.first" + name: "first" + permission: READ_ONLY +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("Message not initialized")); + + // bad input: missing permission in flag value + let error = flag_values::try_from_text_proto( + r#" +flag_value { + package: "com.first" + name: "first" + state: DISABLED +} +"#, + ) + .unwrap_err(); + assert!(format!("{:?}", error).contains("Message not initialized")); + } + + fn try_from_binary_proto_from_text_proto(text_proto: &str) -> Result { + use protobuf::Message; + + let parsed_flags: ProtoParsedFlags = try_from_text_proto(text_proto)?; + let mut binary_proto = Vec::new(); + parsed_flags.write_to_vec(&mut binary_proto)?; + parsed_flags::try_from_binary_proto(&binary_proto) + } + + #[test] + fn test_parsed_flags_try_from_text_proto() { + // valid input + let text_proto = r#" +parsed_flag { + package: "com.first" + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +parsed_flag { + package: "com.second" + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } + trace { + source: "flags.values" + state: ENABLED + permission: READ_WRITE + } +} +"#; + let parsed_flags = try_from_binary_proto_from_text_proto(text_proto).unwrap(); + assert_eq!(parsed_flags.parsed_flag.len(), 2); + let second = parsed_flags.parsed_flag.iter().find(|fv| fv.name() == "second").unwrap(); + assert_eq!(second.package(), "com.second"); + assert_eq!(second.name(), "second"); + assert_eq!(second.namespace(), "second_ns"); + assert_eq!(second.description(), "This is the description of the second flag."); + assert_eq!(second.state(), ProtoFlagState::ENABLED); + assert_eq!(second.permission(), ProtoFlagPermission::READ_WRITE); + assert_eq!(2, second.trace.len()); + assert_eq!(second.trace[0].source(), "flags.declarations"); + assert_eq!(second.trace[0].state(), ProtoFlagState::DISABLED); + assert_eq!(second.trace[0].permission(), ProtoFlagPermission::READ_ONLY); + assert_eq!(second.trace[1].source(), "flags.values"); + assert_eq!(second.trace[1].state(), ProtoFlagState::ENABLED); + assert_eq!(second.trace[1].permission(), ProtoFlagPermission::READ_WRITE); + + // valid input: empty + let parsed_flags = try_from_binary_proto_from_text_proto("").unwrap(); + assert!(parsed_flags.parsed_flag.is_empty()); + + // bad input: empty trace + let text_proto = r#" +parsed_flag { + package: "com.first" + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY +} +"#; + let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); + assert_eq!(format!("{:?}", error), "bad parsed flag: empty trace"); + + // bad input: missing fields in parsed_flag + let text_proto = r#" +parsed_flag { + package: "com.first" + name: "first" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); + assert!(format!("{:?}", error).contains("Message not initialized")); + + // bad input: parsed_flag not sorted by package + let text_proto = r#" +parsed_flag { + package: "bbb" + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +parsed_flag { + package: "aaa" + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); + assert_eq!( + format!("{:?}", error), + "bad parsed flags: not sorted: bbb.first comes before aaa.second" + ); + + // bad input: parsed_flag not sorted by name + let text_proto = r#" +parsed_flag { + package: "com.foo" + name: "bbb" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +parsed_flag { + package: "com.foo" + name: "aaa" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); + assert_eq!( + format!("{:?}", error), + "bad parsed flags: not sorted: com.foo.bbb comes before com.foo.aaa" + ); + + // bad input: duplicate flags + let text_proto = r#" +parsed_flag { + package: "com.foo" + name: "bar" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +parsed_flag { + package: "com.foo" + name: "bar" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let error = try_from_binary_proto_from_text_proto(text_proto).unwrap_err(); + assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.foo.bar"); + } + + #[test] + fn test_parsed_flags_merge() { + let text_proto = r#" +parsed_flag { + package: "com.first" + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +parsed_flag { + package: "com.second" + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let expected = try_from_binary_proto_from_text_proto(text_proto).unwrap(); + + let text_proto = r#" +parsed_flag { + package: "com.first" + name: "first" + namespace: "first_ns" + description: "This is the description of the first flag." + state: DISABLED + permission: READ_ONLY + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let first = try_from_binary_proto_from_text_proto(text_proto).unwrap(); + + let text_proto = r#" +parsed_flag { + package: "com.second" + name: "second" + namespace: "second_ns" + description: "This is the description of the second flag." + state: ENABLED + permission: READ_WRITE + trace { + source: "flags.declarations" + state: DISABLED + permission: READ_ONLY + } +} +"#; + let second = try_from_binary_proto_from_text_proto(text_proto).unwrap(); + + // bad cases + let error = parsed_flags::merge(vec![first.clone(), first.clone()]).unwrap_err(); + assert_eq!(format!("{:?}", error), "bad parsed flags: duplicate flag com.first.first"); + + // valid cases + assert!(parsed_flags::merge(vec![]).unwrap().parsed_flag.is_empty()); + assert_eq!(first, parsed_flags::merge(vec![first.clone()]).unwrap()); + assert_eq!(expected, parsed_flags::merge(vec![first.clone(), second.clone()]).unwrap()); + assert_eq!(expected, parsed_flags::merge(vec![second, first]).unwrap()); + } +} diff --git a/tools/aconfig/src/test.rs b/tools/aconfig/src/test.rs index 9d29083dac..abe9015027 100644 --- a/tools/aconfig/src/test.rs +++ b/tools/aconfig/src/test.rs @@ -16,29 +16,32 @@ #[cfg(test)] pub mod test_utils { - use crate::cache::Cache; - use crate::commands::{Input, Source}; + use crate::commands::Input; + use crate::protos::ProtoParsedFlags; use itertools; - pub fn create_cache() -> Cache { - crate::commands::create_cache( + pub const TEST_PACKAGE: &str = "com.android.aconfig.test"; + + pub fn parse_test_flags() -> ProtoParsedFlags { + let bytes = crate::commands::parse_flags( "com.android.aconfig.test", vec![Input { - source: Source::File("tests/test.aconfig".to_string()), + source: "tests/test.aconfig".to_string(), reader: Box::new(include_bytes!("../tests/test.aconfig").as_slice()), }], vec![ Input { - source: Source::File("tests/first.values".to_string()), + source: "tests/first.values".to_string(), reader: Box::new(include_bytes!("../tests/first.values").as_slice()), }, Input { - source: Source::File("tests/second.values".to_string()), + source: "tests/second.values".to_string(), reader: Box::new(include_bytes!("../tests/second.values").as_slice()), }, ], ) - .unwrap() + .unwrap(); + crate::protos::parsed_flags::try_from_binary_proto(&bytes).unwrap() } pub fn first_significant_code_diff(a: &str, b: &str) -> Option { diff --git a/tools/aconfig/templates/rust.template b/tools/aconfig/templates/rust.template index d9149438e6..960c494942 100644 --- a/tools/aconfig/templates/rust.template +++ b/tools/aconfig/templates/rust.template @@ -1,25 +1,25 @@ {{- for mod in modules -}} pub mod {mod} \{ {{ endfor -}} -{{- for parsed_flag in parsed_flags -}} -{{- if parsed_flag.is_read_only_disabled -}} +{{- for flag in template_flags -}} +{{- if flag.is_read_only_disabled -}} #[inline(always)] -pub const fn r#{parsed_flag.name}() -> bool \{ +pub const fn r#{flag.name}() -> bool \{ false } {{ endif -}} -{{- if parsed_flag.is_read_only_enabled -}} +{{- if flag.is_read_only_enabled -}} #[inline(always)] -pub const fn r#{parsed_flag.name}() -> bool \{ +pub const fn r#{flag.name}() -> bool \{ true } {{ endif -}} -{{- if parsed_flag.is_read_write -}} +{{- if flag.is_read_write -}} #[inline(always)] -pub fn r#{parsed_flag.name}() -> bool \{ - flags_rust::GetServerConfigurableFlag("{parsed_flag.device_config_namespace}", "{parsed_flag.device_config_flag}", "false") == "true" +pub fn r#{flag.name}() -> bool \{ + flags_rust::GetServerConfigurableFlag("{flag.device_config_namespace}", "{flag.device_config_flag}", "false") == "true" } {{ endif -}}