From d74941b42d7392c062f607394972ee0389981f96 Mon Sep 17 00:00:00 2001 From: Dennis Shen Date: Tue, 9 Apr 2024 20:59:11 +0000 Subject: [PATCH] aconfig: create flag info file write c api Bug: b/312444587 Test: atest -c Change-Id: I310e1ed727ced454bec2016afe48f7a29561fac3 --- tools/aconfig/aconfig_storage_file/src/lib.rs | 20 +++- .../aconfig_storage_read_api.hpp | 40 ++++--- .../aconfig_storage_write_api/Android.bp | 2 + .../aconfig_storage_write_api.cpp | 89 ++++++++++++---- .../aconfig_storage_write_api.hpp | 31 ++++-- .../aconfig_storage_write_api/src/lib.rs | 78 ++++++++++++++ .../tests/storage_write_api_test.cpp | 100 ++++++++++++++++-- 7 files changed, 305 insertions(+), 55 deletions(-) diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs index c73807fdcb..070a3cfc17 100644 --- a/tools/aconfig/aconfig_storage_file/src/lib.rs +++ b/tools/aconfig/aconfig_storage_file/src/lib.rs @@ -51,7 +51,9 @@ 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, InvalidStoredFlagType}; +use crate::AconfigStorageError::{ + BytesParseFail, HashTableSizeLimit, InvalidFlagValueType, InvalidStoredFlagType, +}; /// Storage file version pub const FILE_VERSION: u32 = 1; @@ -125,7 +127,7 @@ impl TryFrom for StoredFlagType { } /// Flag value type enum, one FlagValueType maps to many StoredFlagType -/// ONLY APPEND, NEVER REMOVE FOR BACKWARD COMPATIBILITY. +/// ONLY APPEND, NEVER REMOVE FOR BACKWARD COMPATIBILITY. THE MAX IS U16 #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum FlagValueType { Boolean = 0, @@ -143,6 +145,17 @@ impl TryFrom for FlagValueType { } } +impl TryFrom for FlagValueType { + type Error = AconfigStorageError; + + fn try_from(value: u16) -> Result { + match value { + x if x == Self::Boolean as u16 => Ok(Self::Boolean), + _ => Err(InvalidFlagValueType(anyhow!("Invalid flag value type"))), + } + } +} + /// Storage query api error #[non_exhaustive] #[derive(thiserror::Error, Debug)] @@ -182,6 +195,9 @@ pub enum AconfigStorageError { #[error("invalid stored flag type")] InvalidStoredFlagType(#[source] anyhow::Error), + + #[error("invalid flag value type")] + InvalidFlagValueType(#[source] anyhow::Error), } /// Get the right hash table size given number of entries in the table. Use a diff --git a/tools/aconfig/aconfig_storage_read_api/include/aconfig_storage/aconfig_storage_read_api.hpp b/tools/aconfig/aconfig_storage_read_api/include/aconfig_storage/aconfig_storage_read_api.hpp index 521ff158e8..45552074fc 100644 --- a/tools/aconfig/aconfig_storage_read_api/include/aconfig_storage/aconfig_storage_read_api.hpp +++ b/tools/aconfig/aconfig_storage_read_api/include/aconfig_storage/aconfig_storage_read_api.hpp @@ -6,7 +6,8 @@ namespace aconfig_storage { -/// Storage file type enum +/// Storage file type enum, to be consistent with the one defined in +/// aconfig_storage_file/src/lib.rs enum StorageFileType { package_map, flag_map, @@ -14,6 +15,29 @@ enum StorageFileType { flag_info }; +/// Flag type enum, to be consistent with the one defined in +/// aconfig_storage_file/src/lib.rs +enum StoredFlagType { + ReadWriteBoolean = 0, + ReadOnlyBoolean = 1, + FixedReadOnlyBoolean = 2, +}; + +/// Flag value type enum, to be consistent with the one defined in +/// aconfig_storage_file/src/lib.rs +enum FlagValueType { + Boolean = 0, +}; + +/// Flag info enum, to be consistent with the one defined in +/// aconfig_storage_file/src/flag_info.rs +enum FlagInfoBit { + IsSticky = 1<<0, + IsReadWrite = 1<<1, + HasOverride = 1<<2, +}; + + /// Mapped storage file struct MappedStorageFile { void* file_ptr; @@ -27,13 +51,6 @@ struct PackageReadContext { uint32_t boolean_start_index; }; -/// Flag type enum, to be consistent with the one defined in aconfig_storage_file/src/lib.rs -enum StoredFlagType { - ReadWriteBoolean = 0, - ReadOnlyBoolean = 1, - FixedReadOnlyBoolean = 2, -}; - /// Flag read context query result struct FlagReadContext { bool flag_exists; @@ -91,13 +108,6 @@ android::base::Result get_boolean_flag_value( MappedStorageFile const& file, uint32_t index); -/// Flag info enum, to be consistent with the one defined in aconfig_storage_file/src/lib.rs -enum FlagInfoBit { - IsSticky = 1<<0, - IsReadWrite = 1<<1, - HasOverride = 1<<2, -}; - /// Get boolean flag attribute /// \input file: mapped storage file /// \input index: the boolean flag index in the file diff --git a/tools/aconfig/aconfig_storage_write_api/Android.bp b/tools/aconfig/aconfig_storage_write_api/Android.bp index dfd6a5c86c..4dbdbbfb2f 100644 --- a/tools/aconfig/aconfig_storage_write_api/Android.bp +++ b/tools/aconfig/aconfig_storage_write_api/Android.bp @@ -31,6 +31,7 @@ rust_test_host { defaults: ["aconfig_storage_write_api.defaults"], data: [ "tests/flag.val", + "tests/flag.info", ], rustlibs: [ "libaconfig_storage_read_api", @@ -75,6 +76,7 @@ cc_library_static { whole_static_libs: ["libaconfig_storage_write_api_cxx_bridge"], export_include_dirs: ["include"], static_libs: [ + "libaconfig_storage_read_api_cc", "libaconfig_storage_protos_cc", "libprotobuf-cpp-lite", "libbase", diff --git a/tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp b/tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp index ea88f05e58..01785e12c3 100644 --- a/tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp +++ b/tools/aconfig/aconfig_storage_write_api/aconfig_storage_write_api.cpp @@ -38,7 +38,8 @@ static Result read_storage_records_pb(std::string const& pb_ /// Get storage file path static Result find_storage_file( std::string const& pb_file, - std::string const& container) { + std::string const& container, + StorageFileType file_type) { auto records_pb = read_storage_records_pb(pb_file); if (!records_pb.ok()) { return Error() << "Unable to read storage records from " << pb_file @@ -47,15 +48,26 @@ static Result find_storage_file( for (auto& entry : records_pb->files()) { if (entry.container() == container) { - return entry.flag_val(); + switch(file_type) { + case StorageFileType::package_map: + return entry.package_map(); + case StorageFileType::flag_map: + return entry.flag_map(); + case StorageFileType::flag_val: + return entry.flag_val(); + case StorageFileType::flag_info: + return entry.flag_info(); + default: + return Error() << "Invalid file type " << file_type; + } } } - return Error() << "Unable to find storage files for container " << container;; + return Error() << "Unable to find storage files for container " << container; } /// Map a storage file -static Result map_storage_file(std::string const& file) { +static Result map_storage_file(std::string const& file) { struct stat file_stat; if (stat(file.c_str(), &file_stat) < 0) { return ErrnoError() << "stat failed"; @@ -78,7 +90,7 @@ static Result map_storage_file(std::string const& file) { return ErrnoError() << "mmap failed"; } - auto mapped_file = MappedFlagValueFile(); + auto mapped_file = MutableMappedStorageFile(); mapped_file.file_ptr = map_result; mapped_file.file_size = file_size; @@ -87,34 +99,37 @@ static Result map_storage_file(std::string const& file) { namespace private_internal_api { -/// Get mapped file implementation. -Result get_mapped_flag_value_file_impl( +/// Get mutable mapped file implementation. +Result get_mutable_mapped_file_impl( std::string const& pb_file, - std::string const& container) { - auto file_result = find_storage_file(pb_file, container); + std::string const& container, + StorageFileType file_type) { + if (file_type != StorageFileType::flag_val && + file_type != StorageFileType::flag_info) { + return Error() << "Cannot create mutable mapped file for this file type"; + } + + auto file_result = find_storage_file(pb_file, container, file_type); if (!file_result.ok()) { return Error() << file_result.error(); } - auto mapped_result = map_storage_file(*file_result); - if (!mapped_result.ok()) { - return Error() << "failed to map " << *file_result << ": " - << mapped_result.error(); - } - return *mapped_result; + + return map_storage_file(*file_result); } } // namespace private internal api -/// Get mapped writeable flag value file -Result get_mapped_flag_value_file( - std::string const& container) { - return private_internal_api::get_mapped_flag_value_file_impl( - kPersistStorageRecordsPb, container); +/// Get mutable mapped file +Result get_mutable_mapped_file( + std::string const& container, + StorageFileType file_type) { + return private_internal_api::get_mutable_mapped_file_impl( + kPersistStorageRecordsPb, container, file_type); } /// Set boolean flag value Result set_boolean_flag_value( - const MappedFlagValueFile& file, + const MutableMappedStorageFile& file, uint32_t offset, bool value) { auto content = rust::Slice( @@ -126,6 +141,38 @@ Result set_boolean_flag_value( return {}; } +/// Set if flag is sticky +Result set_flag_is_sticky( + const MutableMappedStorageFile& file, + FlagValueType value_type, + uint32_t offset, + bool value) { + auto content = rust::Slice( + static_cast(file.file_ptr), file.file_size); + auto update_cxx = update_flag_is_sticky_cxx( + content, static_cast(value_type), offset, value); + if (!update_cxx.update_success) { + return Error() << std::string(update_cxx.error_message.c_str()); + } + return {}; +} + +/// Set if flag has override +Result set_flag_has_override( + const MutableMappedStorageFile& file, + FlagValueType value_type, + uint32_t offset, + bool value) { + auto content = rust::Slice( + static_cast(file.file_ptr), file.file_size); + auto update_cxx = update_flag_has_override_cxx( + content, static_cast(value_type), offset, value); + if (!update_cxx.update_success) { + return Error() << std::string(update_cxx.error_message.c_str()); + } + return {}; +} + Result create_flag_info( std::string const& package_map, std::string const& flag_map, diff --git a/tools/aconfig/aconfig_storage_write_api/include/aconfig_storage/aconfig_storage_write_api.hpp b/tools/aconfig/aconfig_storage_write_api/include/aconfig_storage/aconfig_storage_write_api.hpp index b652510a61..8699b88614 100644 --- a/tools/aconfig/aconfig_storage_write_api/include/aconfig_storage/aconfig_storage_write_api.hpp +++ b/tools/aconfig/aconfig_storage_write_api/include/aconfig_storage/aconfig_storage_write_api.hpp @@ -4,13 +4,14 @@ #include #include +#include using namespace android::base; namespace aconfig_storage { /// Mapped flag value file -struct MappedFlagValueFile{ +struct MutableMappedStorageFile{ void* file_ptr; size_t file_size; }; @@ -18,19 +19,35 @@ struct MappedFlagValueFile{ /// DO NOT USE APIS IN THE FOLLOWING NAMESPACE DIRECTLY namespace private_internal_api { -Result get_mapped_flag_value_file_impl( +Result get_mutable_mapped_file_impl( std::string const& pb_file, - std::string const& container); + std::string const& container, + StorageFileType file_type); } // namespace private_internal_api -/// Get mapped writeable flag value file -Result get_mapped_flag_value_file( - std::string const& container); +/// Get mapped writeable storage file +Result get_mutable_mapped_file( + std::string const& container, + StorageFileType file_type); /// Set boolean flag value Result set_boolean_flag_value( - const MappedFlagValueFile& file, + const MutableMappedStorageFile& file, + uint32_t offset, + bool value); + +/// Set if flag is sticky +Result set_flag_is_sticky( + const MutableMappedStorageFile& file, + FlagValueType value_type, + uint32_t offset, + bool value); + +/// Set if flag has override +Result set_flag_has_override( + const MutableMappedStorageFile& file, + FlagValueType value_type, uint32_t offset, bool value); diff --git a/tools/aconfig/aconfig_storage_write_api/src/lib.rs b/tools/aconfig/aconfig_storage_write_api/src/lib.rs index 51312a8472..ee08d1651b 100644 --- a/tools/aconfig/aconfig_storage_write_api/src/lib.rs +++ b/tools/aconfig/aconfig_storage_write_api/src/lib.rs @@ -206,6 +206,18 @@ mod ffi { pub error_message: String, } + // Flag is sticky update return for cc interlop + pub struct FlagIsStickyUpdateCXX { + pub update_success: bool, + pub error_message: String, + } + + // Flag has override update return for cc interlop + pub struct FlagHasOverrideUpdateCXX { + pub update_success: bool, + pub error_message: String, + } + // Flag info file creation return for cc interlop pub struct FlagInfoCreationCXX { pub success: bool, @@ -220,6 +232,20 @@ mod ffi { value: bool, ) -> BooleanFlagValueUpdateCXX; + pub fn update_flag_is_sticky_cxx( + file: &mut [u8], + flag_type: u16, + offset: u32, + value: bool, + ) -> FlagIsStickyUpdateCXX; + + pub fn update_flag_has_override_cxx( + file: &mut [u8], + flag_type: u16, + offset: u32, + value: bool, + ) -> FlagHasOverrideUpdateCXX; + pub fn create_flag_info_cxx( package_map: &str, flag_map: &str, @@ -244,6 +270,58 @@ pub(crate) fn update_boolean_flag_value_cxx( } } +pub(crate) fn update_flag_is_sticky_cxx( + file: &mut [u8], + flag_type: u16, + offset: u32, + value: bool, +) -> ffi::FlagIsStickyUpdateCXX { + 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 { + update_success: true, + error_message: String::from("") + }, + Err(errmsg) => ffi::FlagIsStickyUpdateCXX { + update_success: false, + error_message: format!("{:?}", errmsg), + }, + } + } + Err(errmsg) => ffi::FlagIsStickyUpdateCXX { + update_success: false, + error_message: format!("{:?}", errmsg), + } + } +} + +pub(crate) fn update_flag_has_override_cxx( + file: &mut [u8], + flag_type: u16, + offset: u32, + value: bool, +) -> ffi::FlagHasOverrideUpdateCXX { + 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 { + update_success: true, + error_message: String::from("") + }, + Err(errmsg) => ffi::FlagHasOverrideUpdateCXX { + update_success: false, + error_message: format!("{:?}", errmsg), + }, + } + } + Err(errmsg) => ffi::FlagHasOverrideUpdateCXX { + update_success: false, + error_message: format!("{:?}", errmsg), + } + } +} + /// Create flag info file cc interlop pub(crate) fn create_flag_info_cxx( package_map: &str, diff --git a/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp b/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp index 00b737c029..77664e40a8 100644 --- a/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp +++ b/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.cpp @@ -50,7 +50,8 @@ class AconfigStorageTest : public ::testing::Test { return temp_file; } - Result write_storage_location_pb_file(std::string const& flag_val) { + Result write_storage_location_pb_file(std::string const& flag_val, + std::string const& flag_info) { auto temp_file = std::tmpnam(nullptr); auto proto = storage_files(); auto* info = proto.add_files(); @@ -59,6 +60,7 @@ class AconfigStorageTest : public ::testing::Test { info->set_package_map("some_package.map"); info->set_flag_map("some_flag.map"); info->set_flag_val(flag_val); + info->set_flag_info(flag_info); info->set_timestamp(12345); auto content = std::string(); @@ -72,22 +74,25 @@ class AconfigStorageTest : public ::testing::Test { void SetUp() override { auto const test_dir = android::base::GetExecutableDirectory(); flag_val = *copy_to_rw_temp_file(test_dir + "/flag.val"); - storage_record_pb = *write_storage_location_pb_file(flag_val); + flag_info = *copy_to_rw_temp_file(test_dir + "/flag.info"); + storage_record_pb = *write_storage_location_pb_file(flag_val, flag_info); } void TearDown() override { std::remove(flag_val.c_str()); + std::remove(flag_info.c_str()); std::remove(storage_record_pb.c_str()); } std::string flag_val; + std::string flag_info; std::string storage_record_pb; }; /// Negative test to lock down the error when mapping none exist storage files TEST_F(AconfigStorageTest, test_none_exist_storage_file_mapping) { - auto mapped_file_result = private_api::get_mapped_flag_value_file_impl( - storage_record_pb, "vendor"); + auto mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "vendor", api::StorageFileType::flag_val); ASSERT_FALSE(mapped_file_result.ok()); ASSERT_EQ(mapped_file_result.error().message(), "Unable to find storage files for container vendor"); @@ -96,17 +101,34 @@ TEST_F(AconfigStorageTest, test_none_exist_storage_file_mapping) { /// Negative test to lock down the error when mapping a non writeable storage file TEST_F(AconfigStorageTest, test_non_writable_storage_file_mapping) { ASSERT_TRUE(chmod(flag_val.c_str(), S_IRUSR | S_IRGRP | S_IROTH) != -1); - auto mapped_file_result = private_api::get_mapped_flag_value_file_impl( - storage_record_pb, "mockup"); + auto mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "mockup", api::StorageFileType::flag_val); ASSERT_FALSE(mapped_file_result.ok()); auto it = mapped_file_result.error().message().find("cannot map nonwriteable file"); ASSERT_TRUE(it != std::string::npos) << mapped_file_result.error().message(); } +/// Negative test to lock down the error when mapping a file type that cannot be modified +TEST_F(AconfigStorageTest, test_invalid_storage_file_type_mapping) { + auto mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "mockup", api::StorageFileType::package_map); + ASSERT_FALSE(mapped_file_result.ok()); + auto it = mapped_file_result.error().message().find( + "Cannot create mutable mapped file for this file type"); + ASSERT_TRUE(it != std::string::npos) << mapped_file_result.error().message(); + + mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "mockup", api::StorageFileType::flag_map); + ASSERT_FALSE(mapped_file_result.ok()); + it = mapped_file_result.error().message().find( + "Cannot create mutable mapped file for this file type"); + ASSERT_TRUE(it != std::string::npos) << mapped_file_result.error().message(); +} + /// Test to lock down storage flag value update api TEST_F(AconfigStorageTest, test_boolean_flag_value_update) { - auto mapped_file_result = private_api::get_mapped_flag_value_file_impl( - storage_record_pb, "mockup"); + auto mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "mockup", api::StorageFileType::flag_val); ASSERT_TRUE(mapped_file_result.ok()); auto mapped_file = *mapped_file_result; @@ -124,8 +146,8 @@ TEST_F(AconfigStorageTest, test_boolean_flag_value_update) { /// Negative test to lock down the error when querying flag value out of range TEST_F(AconfigStorageTest, test_invalid_boolean_flag_value_update) { - auto mapped_file_result = private_api::get_mapped_flag_value_file_impl( - storage_record_pb, "mockup"); + auto mapped_file_result = private_api::get_mutable_mapped_file_impl( + storage_record_pb, "mockup", api::StorageFileType::flag_val); ASSERT_TRUE(mapped_file_result.ok()); auto mapped_file = *mapped_file_result; auto update_result = api::set_boolean_flag_value(mapped_file, 8, true); @@ -133,3 +155,61 @@ TEST_F(AconfigStorageTest, test_invalid_boolean_flag_value_update) { ASSERT_EQ(update_result.error().message(), 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) { + 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( + mapped_file, api::FlagValueType::Boolean, offset, true); + ASSERT_TRUE(update_result.ok()); + auto ro_mapped_file = api::MappedStorageFile(); + ro_mapped_file.file_ptr = mapped_file.file_ptr; + ro_mapped_file.file_size = mapped_file.file_size; + auto attribute = api::get_boolean_flag_attribute(ro_mapped_file, offset); + ASSERT_TRUE(attribute.ok()); + ASSERT_TRUE(*attribute & api::FlagInfoBit::IsSticky); + + update_result = api::set_flag_is_sticky( + mapped_file, api::FlagValueType::Boolean, offset, false); + ASSERT_TRUE(update_result.ok()); + ro_mapped_file.file_ptr = mapped_file.file_ptr; + ro_mapped_file.file_size = mapped_file.file_size; + attribute = api::get_boolean_flag_attribute(ro_mapped_file, offset); + ASSERT_TRUE(attribute.ok()); + ASSERT_FALSE(*attribute & api::FlagInfoBit::IsSticky); + } +} + +/// Test to lock down storage flag has override update api +TEST_F(AconfigStorageTest, test_flag_has_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( + mapped_file, api::FlagValueType::Boolean, offset, true); + ASSERT_TRUE(update_result.ok()); + auto ro_mapped_file = api::MappedStorageFile(); + ro_mapped_file.file_ptr = mapped_file.file_ptr; + ro_mapped_file.file_size = mapped_file.file_size; + auto attribute = api::get_boolean_flag_attribute(ro_mapped_file, offset); + ASSERT_TRUE(attribute.ok()); + ASSERT_TRUE(*attribute & api::FlagInfoBit::HasOverride); + + update_result = api::set_flag_has_override( + mapped_file, api::FlagValueType::Boolean, offset, false); + ASSERT_TRUE(update_result.ok()); + ro_mapped_file.file_ptr = mapped_file.file_ptr; + ro_mapped_file.file_size = mapped_file.file_size; + attribute = api::get_boolean_flag_attribute(ro_mapped_file, offset); + ASSERT_TRUE(attribute.ok()); + ASSERT_FALSE(*attribute & api::FlagInfoBit::HasOverride); + } +}