Merge "aconfig: use an enum to represetn flag type" into main

This commit is contained in:
Dennis Shen
2024-04-01 20:15:03 +00:00
committed by Gerrit Code Review
10 changed files with 120 additions and 29 deletions

View File

@@ -16,8 +16,10 @@
use crate::commands::assign_flag_ids;
use crate::storage::FlagPackage;
use aconfig_protos::ProtoFlagPermission;
use aconfig_storage_file::{
get_table_size, FlagTable, FlagTableHeader, FlagTableNode, StorageFileType, FILE_VERSION,
get_table_size, FlagTable, FlagTableHeader, FlagTableNode, StorageFileType, StoredFlagType,
FILE_VERSION,
};
use anyhow::{anyhow, Result};
@@ -45,7 +47,7 @@ impl FlagTableNodeWrapper {
fn new(
package_id: u32,
flag_name: &str,
flag_type: u16,
flag_type: StoredFlagType,
flag_id: u16,
num_buckets: u32,
) -> Self {
@@ -70,11 +72,14 @@ impl FlagTableNodeWrapper {
let fid = flag_ids
.get(pf.name())
.ok_or(anyhow!(format!("missing flag id for {}", pf.name())))?;
// all flags are boolean value at the moment, thus using the last bit.
// When more flag value types are supported, flag value type information
// should come from the parsed flag, and we will set the flag_type bit
// mask properly.
let flag_type = 1;
let flag_type = if pf.is_fixed_read_only() {
StoredFlagType::FixedReadOnlyBoolean
} else {
match pf.permission() {
ProtoFlagPermission::READ_WRITE => StoredFlagType::ReadWriteBoolean,
ProtoFlagPermission::READ_ONLY => StoredFlagType::ReadOnlyBoolean,
}
};
Ok(Self::new(package.package_id, pf.name(), flag_type, *fid, num_buckets))
})
.collect::<Result<Vec<_>>>()
@@ -147,7 +152,7 @@ mod tests {
FlagTableNode {
package_id,
flag_name: flag_name.to_string(),
flag_type,
flag_type: StoredFlagType::try_from(flag_type).unwrap(),
flag_id,
next_offset,
}
@@ -203,12 +208,12 @@ mod tests {
assert_eq!(nodes.len(), 8);
assert_eq!(nodes[0], new_expected_node(0, "enabled_ro", 1, 1, None));
assert_eq!(nodes[1], new_expected_node(0, "enabled_rw", 1, 2, Some(151)));
assert_eq!(nodes[1], new_expected_node(0, "enabled_rw", 0, 2, Some(151)));
assert_eq!(nodes[2], new_expected_node(1, "disabled_ro", 1, 0, None));
assert_eq!(nodes[3], new_expected_node(2, "enabled_ro", 1, 1, None));
assert_eq!(nodes[4], new_expected_node(1, "enabled_fixed_ro", 1, 1, Some(236)));
assert_eq!(nodes[4], new_expected_node(1, "enabled_fixed_ro", 2, 1, Some(236)));
assert_eq!(nodes[5], new_expected_node(1, "enabled_ro", 1, 2, None));
assert_eq!(nodes[6], new_expected_node(2, "enabled_fixed_ro", 1, 0, None));
assert_eq!(nodes[7], new_expected_node(0, "disabled_rw", 1, 0, None));
assert_eq!(nodes[6], new_expected_node(2, "enabled_fixed_ro", 2, 0, None));
assert_eq!(nodes[7], new_expected_node(0, "disabled_rw", 0, 0, None));
}
}

View File

@@ -88,7 +88,7 @@ mod tests {
assert_eq!(header, &expected_header);
let booleans: &Vec<bool> = &flag_value_list.as_ref().unwrap().booleans;
let expected_booleans: Vec<bool> = vec![false; header.num_flags as usize];
let expected_booleans: Vec<bool> = vec![false, true, true, false, true, true, true, true];
assert_eq!(booleans, &expected_booleans);
}
}

View File

@@ -122,30 +122,38 @@ mod tests {
"com.android.aconfig.storage.test_1",
"storage_test_1.aconfig",
include_bytes!("../../tests/storage_test_1.aconfig").as_slice(),
"storage_test_1.value",
include_bytes!("../../tests/storage_test_1.values").as_slice(),
),
(
"com.android.aconfig.storage.test_2",
"storage_test_2.aconfig",
include_bytes!("../../tests/storage_test_2.aconfig").as_slice(),
"storage_test_2.value",
include_bytes!("../../tests/storage_test_2.values").as_slice(),
),
(
"com.android.aconfig.storage.test_4",
"storage_test_4.aconfig",
include_bytes!("../../tests/storage_test_4.aconfig").as_slice(),
"storage_test_4.value",
include_bytes!("../../tests/storage_test_4.values").as_slice(),
),
];
aconfig_files
.into_iter()
.map(|(pkg, file, content)| {
.map(|(pkg, aconfig_file, aconfig_content, value_file, value_content)| {
let bytes = crate::commands::parse_flags(
pkg,
Some("system"),
vec![Input {
source: format!("tests/{}", file).to_string(),
reader: Box::new(content),
source: format!("tests/{}", aconfig_file).to_string(),
reader: Box::new(aconfig_content),
}],
vec![Input {
source: format!("tests/{}", value_file).to_string(),
reader: Box::new(value_content),
}],
vec![],
crate::commands::DEFAULT_FLAG_PERMISSION,
)
.unwrap();

View File

@@ -0,0 +1,18 @@
flag_value {
package: "com.android.aconfig.storage.test_1"
name: "enabled_rw"
state: ENABLED
permission: READ_WRITE
}
flag_value {
package: "com.android.aconfig.storage.test_1"
name: "disabled_rw"
state: DISABLED
permission: READ_WRITE
}
flag_value {
package: "com.android.aconfig.storage.test_1"
name: "enabled_ro"
state: ENABLED
permission: READ_ONLY
}

View File

@@ -0,0 +1,18 @@
flag_value {
package: "com.android.aconfig.storage.test_2"
name: "enabled_ro"
state: ENABLED
permission: READ_ONLY
}
flag_value {
package: "com.android.aconfig.storage.test_2"
name: "disabled_ro"
state: DISABLED
permission: READ_ONLY
}
flag_value {
package: "com.android.aconfig.storage.test_2"
name: "enabled_fixed_ro"
state: ENABLED
permission: READ_ONLY
}

View File

@@ -0,0 +1,12 @@
flag_value {
package: "com.android.aconfig.storage.test_4"
name: "enabled_ro"
state: ENABLED
permission: READ_ONLY
}
flag_value {
package: "com.android.aconfig.storage.test_4"
name: "enabled_fixed_ro"
state: ENABLED
permission: READ_ONLY
}

View File

@@ -21,7 +21,7 @@ use crate::{
get_bucket_index, read_str_from_bytes, read_u16_from_bytes, read_u32_from_bytes,
read_u8_from_bytes,
};
use crate::{AconfigStorageError, StorageFileType};
use crate::{AconfigStorageError, StorageFileType, StoredFlagType};
use anyhow::anyhow;
use std::fmt;
@@ -99,7 +99,7 @@ impl FlagTableHeader {
pub struct FlagTableNode {
pub package_id: u32,
pub flag_name: String,
pub flag_type: u16,
pub flag_type: StoredFlagType,
pub flag_id: u16,
pub next_offset: Option<u32>,
}
@@ -109,7 +109,7 @@ impl fmt::Debug for FlagTableNode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
writeln!(
f,
"Package Id: {}, Flag: {}, Type: {}, Offset: {}, Next: {:?}",
"Package Id: {}, Flag: {}, Type: {:?}, Offset: {}, Next: {:?}",
self.package_id, self.flag_name, self.flag_type, self.flag_id, self.next_offset
)?;
Ok(())
@@ -124,7 +124,7 @@ impl FlagTableNode {
let name_bytes = self.flag_name.as_bytes();
result.extend_from_slice(&(name_bytes.len() as u32).to_le_bytes());
result.extend_from_slice(name_bytes);
result.extend_from_slice(&self.flag_type.to_le_bytes());
result.extend_from_slice(&(self.flag_type as u16).to_le_bytes());
result.extend_from_slice(&self.flag_id.to_le_bytes());
result.extend_from_slice(&self.next_offset.unwrap_or(0).to_le_bytes());
result
@@ -136,7 +136,7 @@ impl FlagTableNode {
let node = Self {
package_id: read_u32_from_bytes(bytes, &mut head)?,
flag_name: read_str_from_bytes(bytes, &mut head)?,
flag_type: read_u16_from_bytes(bytes, &mut head)?,
flag_type: StoredFlagType::try_from(read_u16_from_bytes(bytes, &mut head)?)?,
flag_id: read_u16_from_bytes(bytes, &mut head)?,
next_offset: match read_u32_from_bytes(bytes, &mut head)? {
0 => None,

View File

@@ -52,7 +52,7 @@ pub use crate::flag_table::{FlagTable, FlagTableHeader, FlagTableNode};
pub use crate::flag_value::{FlagValueHeader, FlagValueList};
pub use crate::package_table::{PackageTable, PackageTableHeader, PackageTableNode};
use crate::AconfigStorageError::{BytesParseFail, HashTableSizeLimit};
use crate::AconfigStorageError::{BytesParseFail, HashTableSizeLimit, InvalidStoredFlagType};
/// Storage file version
pub const FILE_VERSION: u32 = 1;
@@ -83,7 +83,7 @@ impl TryFrom<&str> for StorageFileType {
"flag_val" => Ok(Self::FlagVal),
"flag_info" => Ok(Self::FlagInfo),
_ => Err(anyhow!(
"Invalid storage file type, valid types are package_map|flag_map|flag_val"
"Invalid storage file type, valid types are package_map|flag_map|flag_val|flag_info"
)),
}
}
@@ -103,6 +103,27 @@ impl TryFrom<u8> for StorageFileType {
}
}
/// Flag type enum as stored by storage file
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum StoredFlagType {
ReadWriteBoolean = 0,
ReadOnlyBoolean = 1,
FixedReadOnlyBoolean = 2,
}
impl TryFrom<u16> for StoredFlagType {
type Error = AconfigStorageError;
fn try_from(value: u16) -> Result<Self, Self::Error> {
match value {
x if x == Self::ReadWriteBoolean as u16 => Ok(Self::ReadWriteBoolean),
x if x == Self::ReadOnlyBoolean as u16 => Ok(Self::ReadOnlyBoolean),
x if x == Self::FixedReadOnlyBoolean as u16 => Ok(Self::FixedReadOnlyBoolean),
_ => Err(InvalidStoredFlagType(anyhow!("Invalid stored flag type"))),
}
}
}
/// Get the right hash table size given number of entries in the table. Use a
/// load factor of 0.5 for performance.
pub fn get_table_size(entries: u32) -> Result<u32, AconfigStorageError> {
@@ -201,6 +222,9 @@ pub enum AconfigStorageError {
#[error("failed to create file")]
FileCreationFail(#[source] anyhow::Error),
#[error("invalid stored flag type")]
InvalidStoredFlagType(#[source] anyhow::Error),
}
/// Read in storage file as bytes

View File

@@ -18,7 +18,7 @@ use crate::flag_info::{FlagInfoHeader, FlagInfoList, FlagInfoNode};
use crate::flag_table::{FlagTable, FlagTableHeader, FlagTableNode};
use crate::flag_value::{FlagValueHeader, FlagValueList};
use crate::package_table::{PackageTable, PackageTableHeader, PackageTableNode};
use crate::{AconfigStorageError, StorageFileType};
use crate::{AconfigStorageError, StorageFileType, StoredFlagType};
use anyhow::anyhow;
use std::io::Write;
@@ -66,7 +66,13 @@ impl FlagTableNode {
flag_id: u16,
next_offset: Option<u32>,
) -> Self {
Self { package_id, flag_name: flag_name.to_string(), flag_type, flag_id, next_offset }
Self {
package_id,
flag_name: flag_name.to_string(),
flag_type: StoredFlagType::try_from(flag_type).unwrap(),
flag_id,
next_offset,
}
}
}

View File

@@ -65,7 +65,7 @@ pub fn find_flag_offset(
#[cfg(test)]
mod tests {
use super::*;
use aconfig_storage_file::{FlagTable, StorageFileType};
use aconfig_storage_file::{FlagTable, StorageFileType, StoredFlagType};
// create test baseline, syntactic sugar
fn new_expected_node(
@@ -78,7 +78,7 @@ mod tests {
FlagTableNode {
package_id,
flag_name: flag_name.to_string(),
flag_type,
flag_type: StoredFlagType::try_from(flag_type).unwrap(),
flag_id,
next_offset,
}