From a4294b0c6a900aadada96ba2210dadd99b4b075a Mon Sep 17 00:00:00 2001 From: Dennis Shen Date: Thu, 25 Jan 2024 15:26:28 +0000 Subject: [PATCH] aconfig: create unit test targets for aconfig_protos and aconfig_storage_file crate also added a bunch of comments to satisfy android lint requirements Bug: b/321077378 Test: atest aconfig.test, atest aconfig_protos.test, atest aconfig_storage_files.test Change-Id: I9bce302ac3bc98c5347e5334d915e77337ee89c4 --- tools/aconfig/TEST_MAPPING | 8 +++++ tools/aconfig/aconfig_protos/Android.bp | 23 +++++++++++---- tools/aconfig/aconfig_protos/src/lib.rs | 29 +++++++++++++++++++ tools/aconfig/aconfig_storage_file/Android.bp | 22 ++++++++++---- .../aconfig_storage_file/src/flag_table.rs | 12 ++++++++ .../aconfig_storage_file/src/flag_value.rs | 9 ++++++ tools/aconfig/aconfig_storage_file/src/lib.rs | 6 ++++ .../aconfig_storage_file/src/package_table.rs | 12 ++++++++ 8 files changed, 111 insertions(+), 10 deletions(-) diff --git a/tools/aconfig/TEST_MAPPING b/tools/aconfig/TEST_MAPPING index de8d93299d..650c8c05be 100644 --- a/tools/aconfig/TEST_MAPPING +++ b/tools/aconfig/TEST_MAPPING @@ -58,6 +58,14 @@ { // printflags unit tests "name": "printflags.test" + }, + { + // aconfig_protos unit tests + "name": "aconfig_protos.test" + }, + { + // aconfig_storage_file unit tests + "name": "aconfig_storage_file.test" } ] } diff --git a/tools/aconfig/aconfig_protos/Android.bp b/tools/aconfig/aconfig_protos/Android.bp index 1cc4e419e2..18c545ae96 100644 --- a/tools/aconfig/aconfig_protos/Android.bp +++ b/tools/aconfig/aconfig_protos/Android.bp @@ -45,12 +45,12 @@ rust_protobuf { host_supported: true, } -rust_library { - name: "libaconfig_protos", +rust_defaults { + name: "aconfig_protos.defaults", + edition: "2021", + clippy_lints: "android", + lints: "android", srcs: ["src/lib.rs"], - crate_name: "aconfig_protos", - host_supported: true, - lints: "none", rustlibs: [ "libaconfig_rust_proto", "libanyhow", @@ -60,3 +60,16 @@ rust_library { "libpaste", ] } + +rust_library { + name: "libaconfig_protos", + crate_name: "aconfig_protos", + host_supported: true, + defaults: ["aconfig_protos.defaults"], +} + +rust_test_host { + name: "aconfig_protos.test", + test_suites: ["general-tests"], + defaults: ["aconfig_protos.defaults"], +} diff --git a/tools/aconfig/aconfig_protos/src/lib.rs b/tools/aconfig/aconfig_protos/src/lib.rs index f0d27d6d47..ef16e06702 100644 --- a/tools/aconfig/aconfig_protos/src/lib.rs +++ b/tools/aconfig/aconfig_protos/src/lib.rs @@ -14,6 +14,7 @@ * limitations under the License. */ +//! `aconfig_protos` is a crate for the protos defined for aconfig // When building with the Android tool-chain // // - an external crate `aconfig_protos` will be generated @@ -68,6 +69,7 @@ pub use auto_generated::*; use anyhow::Result; use paste::paste; +/// Check if the name identifier is valid pub fn is_valid_name_ident(s: &str) -> bool { // Identifiers must match [a-z][a-z0-9_]*, except consecutive underscores are not allowed if s.contains("__") { @@ -83,6 +85,7 @@ pub fn is_valid_name_ident(s: &str) -> bool { chars.all(|ch| ch.is_ascii_lowercase() || ch.is_ascii_digit() || ch == '_') } +/// Check if the package identifier is valid pub fn is_valid_package_ident(s: &str) -> bool { if !s.contains('.') { return false; @@ -90,6 +93,7 @@ pub fn is_valid_package_ident(s: &str) -> bool { s.split('.').all(is_valid_name_ident) } +/// Check if the container identifier is valid pub fn is_valid_container_ident(s: &str) -> bool { s.split('.').all(is_valid_name_ident) } @@ -111,10 +115,12 @@ macro_rules! ensure_required_fields { }; } +/// Utility module for flag_declaration proto pub mod flag_declaration { use super::*; use anyhow::ensure; + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(pdf: &ProtoFlagDeclaration) -> Result<()> { ensure_required_fields!("flag declaration", pdf, "name", "namespace", "description"); @@ -127,16 +133,19 @@ pub mod flag_declaration { } } +/// Utility module for flag_declarations proto pub mod flag_declarations { use super::*; use anyhow::ensure; + /// Construct a proto instance from a textproto string content pub fn try_from_text_proto(s: &str) -> Result { let pdf: ProtoFlagDeclarations = super::try_from_text_proto(s)?; verify_fields(&pdf)?; Ok(pdf) } + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(pdf: &ProtoFlagDeclarations) -> Result<()> { ensure_required_fields!("flag declarations", pdf, "package"); // TODO(b/312769710): Make the container field required. @@ -157,10 +166,12 @@ pub mod flag_declarations { } } +/// Utility module for flag_value proto pub mod flag_value { use super::*; use anyhow::ensure; + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(fv: &ProtoFlagValue) -> Result<()> { ensure_required_fields!("flag value", fv, "package", "name", "state", "permission"); @@ -171,15 +182,18 @@ pub mod flag_value { } } +/// Utility module for flag_values proto pub mod flag_values { use super::*; + /// Construct a proto instance from a textproto string content pub fn try_from_text_proto(s: &str) -> Result { let pfv: ProtoFlagValues = super::try_from_text_proto(s)?; verify_fields(&pfv)?; Ok(pfv) } + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(pfv: &ProtoFlagValues) -> Result<()> { for flag_value in pfv.flag_value.iter() { super::flag_value::verify_fields(flag_value)?; @@ -188,10 +202,12 @@ pub mod flag_values { } } +/// Utility module for flag_permission proto enum pub mod flag_permission { use super::*; use anyhow::bail; + /// Construct a flag permission proto enum from string pub fn parse_from_str(permission: &str) -> Result { match permission.to_ascii_lowercase().as_str() { "read_write" => Ok(ProtoFlagPermission::READ_WRITE), @@ -200,6 +216,7 @@ pub mod flag_permission { } } + /// Serialize flag permission proto enum to string pub fn to_string(permission: &ProtoFlagPermission) -> &str { match permission { ProtoFlagPermission::READ_WRITE => "read_write", @@ -208,10 +225,12 @@ pub mod flag_permission { } } +/// Utility module for tracepoint proto pub mod tracepoint { use super::*; use anyhow::ensure; + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(tp: &ProtoTracepoint) -> Result<()> { ensure_required_fields!("tracepoint", tp, "source", "state", "permission"); @@ -221,10 +240,12 @@ pub mod tracepoint { } } +/// Utility module for parsed_flag proto pub mod parsed_flag { use super::*; use anyhow::ensure; + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(pf: &ProtoParsedFlag) -> Result<()> { ensure_required_fields!( "parsed flag", @@ -265,23 +286,27 @@ pub mod parsed_flag { Ok(()) } + /// Get the file path of the corresponding flag declaration pub fn path_to_declaration(pf: &ProtoParsedFlag) -> &str { debug_assert!(!pf.trace.is_empty()); pf.trace[0].source() } } +/// Utility module for parsed_flags proto pub mod parsed_flags { use super::*; use anyhow::bail; use std::cmp::Ordering; + /// Construct a proto instance from a binary proto bytes pub fn try_from_binary_proto(bytes: &[u8]) -> Result { let message: ProtoParsedFlags = protobuf::Message::parse_from_bytes(bytes)?; verify_fields(&message)?; Ok(message) } + /// Ensure the proto instance is valid by checking its fields pub fn verify_fields(pf: &ProtoParsedFlags) -> Result<()> { use crate::parsed_flag::path_to_declaration; @@ -309,6 +334,7 @@ pub mod parsed_flags { Ok(()) } + /// Merge multipe parsed_flags proto pub fn merge(parsed_flags: Vec, dedup: bool) -> Result { let mut merged = ProtoParsedFlags::new(); for mut pfs in parsed_flags.into_iter() { @@ -325,6 +351,7 @@ pub mod parsed_flags { Ok(merged) } + /// Sort parsed flags pub fn sort_parsed_flags(pf: &mut ProtoParsedFlags) { pf.parsed_flag.sort_by_key(create_sorting_key); } @@ -334,7 +361,9 @@ pub mod parsed_flags { } } +/// ParsedFlagExt trait pub trait ParsedFlagExt { + /// Return the fully qualified name fn fully_qualified_name(&self) -> String; } diff --git a/tools/aconfig/aconfig_storage_file/Android.bp b/tools/aconfig/aconfig_storage_file/Android.bp index 98d7bebd94..13072c9ccf 100644 --- a/tools/aconfig/aconfig_storage_file/Android.bp +++ b/tools/aconfig/aconfig_storage_file/Android.bp @@ -2,13 +2,25 @@ package { default_applicable_licenses: ["Android-Apache-2.0"], } -rust_library { - name: "libaconfig_storage_file", - srcs: ["src/lib.rs"], - crate_name: "aconfig_storage_file", - host_supported: true, +rust_defaults { + name: "aconfig_storage_file.defaults", + edition: "2021", lints: "none", + srcs: ["src/lib.rs"], rustlibs: [ "libanyhow", ], } + +rust_library { + name: "libaconfig_storage_file", + crate_name: "aconfig_storage_file", + host_supported: true, + defaults: ["aconfig_storage_file.defaults"], +} + +rust_test_host { + name: "aconfig_storage_file.test", + test_suites: ["general-tests"], + defaults: ["aconfig_storage_file.defaults"], +} diff --git a/tools/aconfig/aconfig_storage_file/src/flag_table.rs b/tools/aconfig/aconfig_storage_file/src/flag_table.rs index 27a97d1255..dab752a006 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_table.rs @@ -14,9 +14,13 @@ * limitations under the License. */ +//! flag table module defines the flag table file format and methods for serialization +//! and deserialization + use crate::{read_str_from_bytes, read_u16_from_bytes, read_u32_from_bytes}; use anyhow::Result; +/// Flag table header struct #[derive(PartialEq, Debug)] pub struct FlagTableHeader { pub version: u32, @@ -28,6 +32,7 @@ pub struct FlagTableHeader { } impl FlagTableHeader { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { let mut result = Vec::new(); result.extend_from_slice(&self.version.to_le_bytes()); @@ -41,6 +46,7 @@ impl FlagTableHeader { result } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let mut head = 0; Ok(Self { @@ -54,6 +60,7 @@ impl FlagTableHeader { } } +/// Flag table node struct #[derive(PartialEq, Debug, Clone)] pub struct FlagTableNode { pub package_id: u32, @@ -65,6 +72,7 @@ pub struct FlagTableNode { } impl FlagTableNode { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { let mut result = Vec::new(); result.extend_from_slice(&self.package_id.to_le_bytes()); @@ -77,6 +85,7 @@ impl FlagTableNode { result } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8], num_buckets: u32) -> Result { let mut head = 0; let mut node = Self { @@ -103,7 +112,9 @@ pub struct FlagTable { pub nodes: Vec, } +/// Flag table struct impl FlagTable { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { [ self.header.as_bytes(), @@ -113,6 +124,7 @@ impl FlagTable { .concat() } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let header = FlagTableHeader::from_bytes(bytes)?; let num_flags = header.num_flags; diff --git a/tools/aconfig/aconfig_storage_file/src/flag_value.rs b/tools/aconfig/aconfig_storage_file/src/flag_value.rs index 9679c55bf2..86f75ce62a 100644 --- a/tools/aconfig/aconfig_storage_file/src/flag_value.rs +++ b/tools/aconfig/aconfig_storage_file/src/flag_value.rs @@ -14,9 +14,13 @@ * limitations under the License. */ +//! flag value module defines the flag value file format and methods for serialization +//! and deserialization + use crate::{read_str_from_bytes, read_u32_from_bytes, read_u8_from_bytes}; use anyhow::Result; +/// Flag value header struct #[derive(PartialEq, Debug)] pub struct FlagValueHeader { pub version: u32, @@ -27,6 +31,7 @@ pub struct FlagValueHeader { } impl FlagValueHeader { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { let mut result = Vec::new(); result.extend_from_slice(&self.version.to_le_bytes()); @@ -39,6 +44,7 @@ impl FlagValueHeader { result } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let mut head = 0; Ok(Self { @@ -51,6 +57,7 @@ impl FlagValueHeader { } } +/// Flag value list struct #[derive(PartialEq, Debug)] pub struct FlagValueList { pub header: FlagValueHeader, @@ -58,6 +65,7 @@ pub struct FlagValueList { } impl FlagValueList { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { [ self.header.as_bytes(), @@ -66,6 +74,7 @@ impl FlagValueList { .concat() } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let header = FlagValueHeader::from_bytes(bytes)?; let num_flags = header.num_flags; diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs index 078b5aaba3..47fcc9075c 100644 --- a/tools/aconfig/aconfig_storage_file/src/lib.rs +++ b/tools/aconfig/aconfig_storage_file/src/lib.rs @@ -14,6 +14,9 @@ * limitations under the License. */ +//! `aconfig_storage_file` is a crate that defines aconfig storage file format, it +//! also includes apis to read flags from storage files + pub mod flag_table; pub mod flag_value; pub mod package_table; @@ -26,14 +29,17 @@ pub use crate::flag_table::{FlagTable, FlagTableHeader, FlagTableNode}; pub use crate::flag_value::{FlagValueHeader, FlagValueList}; pub use crate::package_table::{PackageTable, PackageTableHeader, PackageTableNode}; +/// Storage file version pub const FILE_VERSION: u32 = 1; +/// Good hash table prime number pub const HASH_PRIMES: [u32; 29] = [ 7, 17, 29, 53, 97, 193, 389, 769, 1543, 3079, 6151, 12289, 24593, 49157, 98317, 196613, 393241, 786433, 1572869, 3145739, 6291469, 12582917, 25165843, 50331653, 100663319, 201326611, 402653189, 805306457, 1610612741, ]; +/// Storage file type enum #[derive(Clone, Debug, PartialEq, Eq)] pub enum StorageFileSelection { PackageMap, diff --git a/tools/aconfig/aconfig_storage_file/src/package_table.rs b/tools/aconfig/aconfig_storage_file/src/package_table.rs index b454430b04..14eef70685 100644 --- a/tools/aconfig/aconfig_storage_file/src/package_table.rs +++ b/tools/aconfig/aconfig_storage_file/src/package_table.rs @@ -14,9 +14,13 @@ * limitations under the License. */ +//! package table module defines the package table file format and methods for serialization +//! and deserialization + use crate::{read_str_from_bytes, read_u32_from_bytes}; use anyhow::Result; +/// Package table header struct #[derive(PartialEq, Debug)] pub struct PackageTableHeader { pub version: u32, @@ -28,6 +32,7 @@ pub struct PackageTableHeader { } impl PackageTableHeader { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { let mut result = Vec::new(); result.extend_from_slice(&self.version.to_le_bytes()); @@ -41,6 +46,7 @@ impl PackageTableHeader { result } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let mut head = 0; Ok(Self { @@ -54,6 +60,7 @@ impl PackageTableHeader { } } +/// Package table node struct #[derive(PartialEq, Debug)] pub struct PackageTableNode { pub package_name: String, @@ -66,6 +73,7 @@ pub struct PackageTableNode { } impl PackageTableNode { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { let mut result = Vec::new(); let name_bytes = self.package_name.as_bytes(); @@ -77,6 +85,7 @@ impl PackageTableNode { result } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8], num_buckets: u32) -> Result { let mut head = 0; let mut node = Self { @@ -94,6 +103,7 @@ impl PackageTableNode { } } +/// Package table struct #[derive(PartialEq, Debug)] pub struct PackageTable { pub header: PackageTableHeader, @@ -102,6 +112,7 @@ pub struct PackageTable { } impl PackageTable { + /// Serialize to bytes pub fn as_bytes(&self) -> Vec { [ self.header.as_bytes(), @@ -111,6 +122,7 @@ impl PackageTable { .concat() } + /// Deserialize from bytes pub fn from_bytes(bytes: &[u8]) -> Result { let header = PackageTableHeader::from_bytes(bytes)?; let num_packages = header.num_packages;