StrixOS/build
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

aconfig: update flag info storage file

Browse Source 
previously we store three bits per flag: is sticky, is read write, has
override. so when a local override arrives, is sticky bit as well has
override bit are set to true. this becomes problematic when the local
override is removed. we are not sure if we should remove has override
bit as well (server override could still be in place).

therefore restructuring the flag info for each flag into new three bits:
is read write, has server override and has local override. so now server
override and local override bits are independent from each other.

Bug: b/312444587
Test: atest -c
Change-Id: I03b2bb7312480773236c95f2b39f2589fee41924
This commit is contained in:
Dennis Shen
2024-04-18 21:11:58 +00:00
parent b24b46d43f
commit 6f4c692948
12 changed files with 116 additions and 113 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp
View File

@@ -142,15 +142,15 @@ Result<void> set_boolean_flag_value(
return {};
}
/// Set if flag is sticky
Result<void> set_flag_is_sticky(
/// Set if flag has server override
Result<void> set_flag_has_server_override(
const MutableMappedStorageFile& file,
FlagValueType value_type,
uint32_t offset,
bool value) {
auto content = rust::Slice<uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
auto update_cxx = update_flag_is_sticky_cxx(
auto update_cxx = update_flag_has_server_override_cxx(
content, static_cast<uint16_t>(value_type), offset, value);
if (!update_cxx.update_success) {
return Error() << std::string(update_cxx.error_message.c_str());
@@ -158,15 +158,15 @@ Result<void> set_flag_is_sticky(
return {};
}
/// Set if flag has override
Result<void> set_flag_has_override(
/// Set if flag has local override
Result<void> set_flag_has_local_override(
const MutableMappedStorageFile& file,
FlagValueType value_type,
uint32_t offset,
bool value) {
auto content = rust::Slice<uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
auto update_cxx = update_flag_has_override_cxx(
auto update_cxx = update_flag_has_local_override_cxx(
content, static_cast<uint16_t>(value_type), offset, value);
if (!update_cxx.update_success) {
return Error() << std::string(update_cxx.error_message.c_str());

8
tools/aconfig/aconfig_storage_write_api/include/aconfig_storage/aconfig_storage_write_api.hpp
View File

@@ -41,15 +41,15 @@ Result<void> set_boolean_flag_value(
uint32_t offset,
bool value);
/// Set if flag is sticky
Result<void> set_flag_is_sticky(
/// Set if flag has server override
Result<void> set_flag_has_server_override(
const MutableMappedStorageFile& file,
FlagValueType value_type,
uint32_t offset,
bool value);
/// Set if flag has override
Result<void> set_flag_has_override(
/// Set if flag has local override
Result<void> set_flag_has_local_override(
const MutableMappedStorageFile& file,
FlagValueType value_type,
uint32_t offset,

42
tools/aconfig/aconfig_storage_write_api/src/flag_info_update.rs
View File

@@ -61,32 +61,32 @@ fn get_flag_attribute_and_offset(
Ok((attribute, head))
}
/// Set if flag is sticky
pub fn update_flag_is_sticky(
/// Set if flag has server override
pub fn update_flag_has_server_override(
buf: &mut [u8],
flag_type: FlagValueType,
flag_index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
let (attribute, head) = get_flag_attribute_and_offset(buf, flag_type, flag_index)?;
let is_sticky = (attribute & (FlagInfoBit::IsSticky as u8)) != 0;
if is_sticky != value {
buf[head] = (attribute ^ FlagInfoBit::IsSticky as u8).to_le_bytes()[0];
let has_override = (attribute & (FlagInfoBit::HasServerOverride as u8)) != 0;
if has_override != value {
buf[head] = (attribute ^ FlagInfoBit::HasServerOverride as u8).to_le_bytes()[0];
}
Ok(())
}
/// Set if flag has override
pub fn update_flag_has_override(
/// Set if flag has local override
pub fn update_flag_has_local_override(
buf: &mut [u8],
flag_type: FlagValueType,
flag_index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
let (attribute, head) = get_flag_attribute_and_offset(buf, flag_type, flag_index)?;
let has_override = (attribute & (FlagInfoBit::HasOverride as u8)) != 0;
let has_override = (attribute & (FlagInfoBit::HasLocalOverride as u8)) != 0;
if has_override != value {
buf[head] = (attribute ^ FlagInfoBit::HasOverride as u8).to_le_bytes()[0];
buf[head] = (attribute ^ FlagInfoBit::HasLocalOverride as u8).to_le_bytes()[0];
}
Ok(())
}
@@ -98,32 +98,32 @@ mod tests {
use aconfig_storage_read_api::flag_info_query::find_flag_attribute;
#[test]
// this test point locks down is sticky update
fn test_update_flag_is_sticky() {
// this test point locks down has server override update
fn test_update_flag_has_server_override() {
let flag_info_list = create_test_flag_info_list();
let mut buf = flag_info_list.into_bytes();
for i in 0..flag_info_list.header.num_flags {
update_flag_is_sticky(&mut buf, FlagValueType::Boolean, i, true).unwrap();
update_flag_has_server_override(&mut buf, FlagValueType::Boolean, i, true).unwrap();
let attribute = find_flag_attribute(&buf, FlagValueType::Boolean, i).unwrap();
assert!((attribute & (FlagInfoBit::IsSticky as u8)) != 0);
update_flag_is_sticky(&mut buf, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) != 0);
update_flag_has_server_override(&mut buf, FlagValueType::Boolean, i, false).unwrap();
let attribute = find_flag_attribute(&buf, FlagValueType::Boolean, i).unwrap();
assert!((attribute & (FlagInfoBit::IsSticky as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) == 0);
}
}
#[test]
// this test point locks down has override update
fn test_update_flag_has_override() {
// this test point locks down has local override update
fn test_update_flag_has_local_override() {
let flag_info_list = create_test_flag_info_list();
let mut buf = flag_info_list.into_bytes();
for i in 0..flag_info_list.header.num_flags {
update_flag_has_override(&mut buf, FlagValueType::Boolean, i, true).unwrap();
update_flag_has_local_override(&mut buf, FlagValueType::Boolean, i, true).unwrap();
let attribute = find_flag_attribute(&buf, FlagValueType::Boolean, i).unwrap();
assert!((attribute & (FlagInfoBit::HasOverride as u8)) != 0);
update_flag_has_override(&mut buf, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) != 0);
update_flag_has_local_override(&mut buf, FlagValueType::Boolean, i, false).unwrap();
let attribute = find_flag_attribute(&buf, FlagValueType::Boolean, i).unwrap();
assert!((attribute & (FlagInfoBit::HasOverride as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) == 0);
}
}
}

81
tools/aconfig/aconfig_storage_write_api/src/lib.rs
View File

@@ -75,39 +75,39 @@ pub fn set_boolean_flag_value(
})
}
/// Set if flag is sticky thru mapped file and flush the change to file
/// Set if flag is has server override thru mapped file and flush the change to file
///
/// \input mapped_file: the mapped flag info file
/// \input index: flag index
/// \input value: updated flag sticky value
/// \input value: updated flag has server override value
/// \return a result of ()
///
pub fn set_flag_is_sticky(
pub fn set_flag_has_server_override(
file: &mut MmapMut,
flag_type: FlagValueType,
index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
crate::flag_info_update::update_flag_is_sticky(file, flag_type, index, value)?;
crate::flag_info_update::update_flag_has_server_override(file, flag_type, index, value)?;
file.flush().map_err(|errmsg| {
AconfigStorageError::MapFlushFail(anyhow!("fail to flush storage file: {}", errmsg))
})
}
/// Set if flag has override thru mapped file and flush the change to file
/// Set if flag has local override thru mapped file and flush the change to file
///
/// \input mapped_file: the mapped flag info file
/// \input index: flag index
/// \input value: updated flag has override value
/// \input value: updated flag has local override value
/// \return a result of ()
///
pub fn set_flag_has_override(
pub fn set_flag_has_local_override(
file: &mut MmapMut,
flag_type: FlagValueType,
index: u32,
value: bool,
) -> Result<(), AconfigStorageError> {
crate::flag_info_update::update_flag_has_override(file, flag_type, index, value)?;
crate::flag_info_update::update_flag_has_local_override(file, flag_type, index, value)?;
file.flush().map_err(|errmsg| {
AconfigStorageError::MapFlushFail(anyhow!("fail to flush storage file: {}", errmsg))
})
@@ -206,14 +206,14 @@ mod ffi {
pub error_message: String,
}
// Flag is sticky update return for cc interlop
pub struct FlagIsStickyUpdateCXX {
// Flag has server override update return for cc interlop
pub struct FlagHasServerOverrideUpdateCXX {
pub update_success: bool,
pub error_message: String,
}
// Flag has override update return for cc interlop
pub struct FlagHasOverrideUpdateCXX {
// Flag has local override update return for cc interlop
pub struct FlagHasLocalOverrideUpdateCXX {
pub update_success: bool,
pub error_message: String,
}
@@ -232,19 +232,19 @@ mod ffi {
value: bool,
) -> BooleanFlagValueUpdateCXX;
pub fn update_flag_is_sticky_cxx(
pub fn update_flag_has_server_override_cxx(
file: &mut [u8],
flag_type: u16,
offset: u32,
value: bool,
) -> FlagIsStickyUpdateCXX;
) -> FlagHasServerOverrideUpdateCXX;
pub fn update_flag_has_override_cxx(
pub fn update_flag_has_local_override_cxx(
file: &mut [u8],
flag_type: u16,
offset: u32,
value: bool,
) -> FlagHasOverrideUpdateCXX;
) -> FlagHasLocalOverrideUpdateCXX;
pub fn create_flag_info_cxx(
package_map: &str,
@@ -270,53 +270,56 @@ pub(crate) fn update_boolean_flag_value_cxx(
}
}
pub(crate) fn update_flag_is_sticky_cxx(
pub(crate) fn update_flag_has_server_override_cxx(
file: &mut [u8],
flag_type: u16,
offset: u32,
value: bool,
) -> ffi::FlagIsStickyUpdateCXX {
) -> ffi::FlagHasServerOverrideUpdateCXX {
match FlagValueType::try_from(flag_type) {
Ok(value_type) => {
match crate::flag_info_update::update_flag_is_sticky(file, value_type, offset, value) {
Ok(()) => ffi::FlagIsStickyUpdateCXX {
match crate::flag_info_update::update_flag_has_server_override(
file, value_type, offset, value,
) {
Ok(()) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: true,
error_message: String::from(""),
},
Err(errmsg) => ffi::FlagIsStickyUpdateCXX {
Err(errmsg) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: false,
error_message: format!("{:?}", errmsg),
},
}
}
Err(errmsg) => ffi::FlagIsStickyUpdateCXX {
Err(errmsg) => ffi::FlagHasServerOverrideUpdateCXX {
update_success: false,
error_message: format!("{:?}", errmsg),
},
}
}
pub(crate) fn update_flag_has_override_cxx(
pub(crate) fn update_flag_has_local_override_cxx(
file: &mut [u8],
flag_type: u16,
offset: u32,
value: bool,
) -> ffi::FlagHasOverrideUpdateCXX {
) -> ffi::FlagHasLocalOverrideUpdateCXX {
match FlagValueType::try_from(flag_type) {
Ok(value_type) => {
match crate::flag_info_update::update_flag_has_override(file, value_type, offset, value)
{
Ok(()) => ffi::FlagHasOverrideUpdateCXX {
match crate::flag_info_update::update_flag_has_local_override(
file, value_type, offset, value,
) {
Ok(()) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: true,
error_message: String::from(""),
},
Err(errmsg) => ffi::FlagHasOverrideUpdateCXX {
Err(errmsg) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: false,
error_message: format!("{:?}", errmsg),
},
}
}
Err(errmsg) => ffi::FlagHasOverrideUpdateCXX {
Err(errmsg) => ffi::FlagHasLocalOverrideUpdateCXX {
update_success: false,
error_message: format!("{:?}", errmsg),
},
@@ -411,7 +414,7 @@ files {{
}
#[test]
fn test_set_flag_is_sticky() {
fn test_set_flag_has_server_override() {
let flag_info_file = copy_to_temp_file("./tests/flag.info", false).unwrap();
let flag_info_path = flag_info_file.path().display().to_string();
let text_proto = format!(
@@ -442,20 +445,20 @@ files {{
)
.unwrap();
for i in 0..8 {
set_flag_is_sticky(&mut file, FlagValueType::Boolean, i, true).unwrap();
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::IsSticky as u8)) != 0);
set_flag_is_sticky(&mut file, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) != 0);
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::IsSticky as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) == 0);
}
}
}
#[test]
fn test_set_flag_has_override() {
fn test_set_flag_has_local_override() {
let flag_info_file = copy_to_temp_file("./tests/flag.info", false).unwrap();
let flag_info_path = flag_info_file.path().display().to_string();
let text_proto = format!(
@@ -486,14 +489,14 @@ files {{
)
.unwrap();
for i in 0..8 {
set_flag_has_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::HasOverride as u8)) != 0);
set_flag_has_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) != 0);
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::HasOverride as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) == 0);
}
}
}

24
tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp
View File

@@ -156,15 +156,15 @@ TEST_F(AconfigStorageTest, test_invalid_boolean_flag_value_update) {
std::string("InvalidStorageFileOffset(Flag value offset goes beyond the end of the file.)"));
}
/// Test to lock down storage flag stickiness update api
TEST_F(AconfigStorageTest, test_flag_is_sticky_update) {
/// Test to lock down storage flag has server override update api
TEST_F(AconfigStorageTest, test_flag_has_server_override_update) {
auto mapped_file_result = private_api::get_mutable_mapped_file_impl(
storage_record_pb, "mockup", api::StorageFileType::flag_info);
ASSERT_TRUE(mapped_file_result.ok());
auto mapped_file = *mapped_file_result;
for (int offset = 0; offset < 8; ++offset) {
auto update_result = api::set_flag_is_sticky(
auto update_result = api::set_flag_has_server_override(
mapped_file, api::FlagValueType::Boolean, offset, true);
ASSERT_TRUE(update_result.ok());
auto ro_mapped_file = api::MappedStorageFile();
@@ -173,9 +173,9 @@ TEST_F(AconfigStorageTest, test_flag_is_sticky_update) {
auto attribute = api::get_flag_attribute(
ro_mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::IsSticky);
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasServerOverride);
update_result = api::set_flag_is_sticky(
update_result = api::set_flag_has_server_override(
mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok());
ro_mapped_file.file_ptr = mapped_file.file_ptr;
@@ -183,19 +183,19 @@ TEST_F(AconfigStorageTest, test_flag_is_sticky_update) {
attribute = api::get_flag_attribute(
ro_mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::IsSticky);
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasServerOverride);
}
}
/// Test to lock down storage flag has override update api
TEST_F(AconfigStorageTest, test_flag_has_override_update) {
/// Test to lock down storage flag has local override update api
TEST_F(AconfigStorageTest, test_flag_has_local_override_update) {
auto mapped_file_result = private_api::get_mutable_mapped_file_impl(
storage_record_pb, "mockup", api::StorageFileType::flag_info);
ASSERT_TRUE(mapped_file_result.ok());
auto mapped_file = *mapped_file_result;
for (int offset = 0; offset < 8; ++offset) {
auto update_result = api::set_flag_has_override(
auto update_result = api::set_flag_has_local_override(
mapped_file, api::FlagValueType::Boolean, offset, true);
ASSERT_TRUE(update_result.ok());
auto ro_mapped_file = api::MappedStorageFile();
@@ -204,9 +204,9 @@ TEST_F(AconfigStorageTest, test_flag_has_override_update) {
auto attribute = api::get_flag_attribute(
ro_mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasOverride);
ASSERT_TRUE(*attribute & api::FlagInfoBit::HasLocalOverride);
update_result = api::set_flag_has_override(
update_result = api::set_flag_has_local_override(
mapped_file, api::FlagValueType::Boolean, offset, false);
ASSERT_TRUE(update_result.ok());
ro_mapped_file.file_ptr = mapped_file.file_ptr;
@@ -214,6 +214,6 @@ TEST_F(AconfigStorageTest, test_flag_has_override_update) {
attribute = api::get_flag_attribute(
ro_mapped_file, api::FlagValueType::Boolean, offset);
ASSERT_TRUE(attribute.ok());
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasOverride);
ASSERT_FALSE(*attribute & api::FlagInfoBit::HasLocalOverride);
}
}

28
tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.rs
View File

@@ -5,8 +5,8 @@ mod aconfig_storage_write_api_test {
use aconfig_storage_read_api::flag_info_query::find_flag_attribute;
use aconfig_storage_read_api::flag_value_query::find_boolean_flag_value;
use aconfig_storage_write_api::{
mapped_file::get_mapped_file, set_boolean_flag_value, set_flag_has_override,
set_flag_is_sticky,
mapped_file::get_mapped_file, set_boolean_flag_value, set_flag_has_local_override,
set_flag_has_server_override,
};
use protobuf::Message;
@@ -90,8 +90,8 @@ files {{
}
#[test]
/// Test to lock down flag is sticky update api
fn test_set_flag_is_sticky() {
/// Test to lock down flag has server override update api
fn test_set_flag_has_server_override() {
let flag_value_file = copy_to_temp_rw_file("./flag.val");
let flag_info_file = copy_to_temp_rw_file("./flag.info");
let flag_value_path = flag_value_file.path().display().to_string();
@@ -106,20 +106,20 @@ files {{
get_mapped_file(&record_pb_path, "mockup", StorageFileType::FlagInfo).unwrap()
};
for i in 0..8 {
set_flag_is_sticky(&mut file, FlagValueType::Boolean, i, true).unwrap();
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::IsSticky as u8)) != 0);
set_flag_is_sticky(&mut file, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) != 0);
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::IsSticky as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasServerOverride as u8)) == 0);
}
}
#[test]
/// Test to lock down flag is sticky update api
fn test_set_flag_has_override() {
/// Test to lock down flag has local override update api
fn test_set_flag_has_local_override() {
let flag_value_file = copy_to_temp_rw_file("./flag.val");
let flag_info_file = copy_to_temp_rw_file("./flag.info");
let flag_value_path = flag_value_file.path().display().to_string();
@@ -134,14 +134,14 @@ files {{
get_mapped_file(&record_pb_path, "mockup", StorageFileType::FlagInfo).unwrap()
};
for i in 0..8 {
set_flag_has_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::HasOverride as u8)) != 0);
set_flag_has_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) != 0);
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, false).unwrap();
let attribute =
get_flag_attribute_at_offset(&flag_info_path, FlagValueType::Boolean, i);
assert!((attribute & (FlagInfoBit::HasOverride as u8)) == 0);
assert!((attribute & (FlagInfoBit::HasLocalOverride as u8)) == 0);
}
}
}