Merge "aconfig: c++ read api to return an android::base::Result and cargo fmt" into main

This commit is contained in:
Treehugger Robot 2024-03-15 14:16:27 +00:00 committed by Gerrit Code Review
commit 6c63da3ce7
9 changed files with 143 additions and 217 deletions

View file

@ -1,6 +1,5 @@
#include <android-base/file.h>
#include <android-base/logging.h>
#include <android-base/result.h>
#include <protos/aconfig_storage_metadata.pb.h>
#include <sys/mman.h>
@ -97,40 +96,21 @@ static Result<MappedStorageFile> map_storage_file(std::string const& file) {
namespace private_internal_api {
/// Get mapped file implementation.
MappedStorageFileQuery get_mapped_file_impl(
Result<MappedStorageFile> get_mapped_file_impl(
std::string const& pb_file,
std::string const& container,
StorageFileType file_type) {
auto query = MappedStorageFileQuery();
auto file_result = find_storage_file(pb_file, container, file_type);
if (!file_result.ok()) {
query.query_success = false;
query.error_message = file_result.error().message();
query.mapped_file.file_ptr = nullptr;
query.mapped_file.file_size = 0;
return query;
return Error() << file_result.error();
}
auto mapped_file_result = map_storage_file(*file_result);
if (!mapped_file_result.ok()) {
query.query_success = false;
query.error_message = mapped_file_result.error().message();
query.mapped_file.file_ptr = nullptr;
query.mapped_file.file_size = 0;
} else {
query.query_success = true;
query.error_message = "";
query.mapped_file = *mapped_file_result;
}
return query;
return map_storage_file(*file_result);
}
} // namespace private internal api
/// Get mapped storage file
MappedStorageFileQuery get_mapped_file(
Result<MappedStorageFile> get_mapped_file(
std::string const& container,
StorageFileType file_type) {
return private_internal_api::get_mapped_file_impl(
@ -138,61 +118,65 @@ MappedStorageFileQuery get_mapped_file(
}
/// Get storage file version number
VersionNumberQuery get_storage_file_version(
Result<uint32_t> get_storage_file_version(
std::string const& file_path) {
auto version_cxx = get_storage_file_version_cxx(
rust::Str(file_path.c_str()));
auto version = VersionNumberQuery();
version.query_success = version_cxx.query_success;
version.error_message = std::string(version_cxx.error_message.c_str());
version.version_number = version_cxx.version_number;
return version;
if (version_cxx.query_success) {
return version_cxx.version_number;
} else {
return Error() << version_cxx.error_message;
}
}
/// Get package offset
PackageOffsetQuery get_package_offset(
Result<PackageOffset> get_package_offset(
MappedStorageFile const& file,
std::string const& package) {
auto content = rust::Slice<const uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
auto offset_cxx = get_package_offset_cxx(content, rust::Str(package.c_str()));
auto offset = PackageOffsetQuery();
offset.query_success = offset_cxx.query_success;
offset.error_message = std::string(offset_cxx.error_message.c_str());
offset.package_exists = offset_cxx.package_exists;
offset.package_id = offset_cxx.package_id;
offset.boolean_offset = offset_cxx.boolean_offset;
return offset;
if (offset_cxx.query_success) {
auto offset = PackageOffset();
offset.package_exists = offset_cxx.package_exists;
offset.package_id = offset_cxx.package_id;
offset.boolean_offset = offset_cxx.boolean_offset;
return offset;
} else {
return Error() << offset_cxx.error_message;
}
}
/// Get flag offset
FlagOffsetQuery get_flag_offset(
Result<FlagOffset> get_flag_offset(
MappedStorageFile const& file,
uint32_t package_id,
std::string const& flag_name){
auto content = rust::Slice<const uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
auto offset_cxx = get_flag_offset_cxx(content, package_id, rust::Str(flag_name.c_str()));
auto offset = FlagOffsetQuery();
offset.query_success = offset_cxx.query_success;
offset.error_message = std::string(offset_cxx.error_message.c_str());
offset.flag_exists = offset_cxx.flag_exists;
offset.flag_offset = offset_cxx.flag_offset;
return offset;
if (offset_cxx.query_success) {
auto offset = FlagOffset();
offset.flag_exists = offset_cxx.flag_exists;
offset.flag_offset = offset_cxx.flag_offset;
return offset;
} else {
return Error() << offset_cxx.error_message;
}
}
/// Get boolean flag value
BooleanFlagValueQuery get_boolean_flag_value(
Result<bool> get_boolean_flag_value(
MappedStorageFile const& file,
uint32_t offset) {
auto content = rust::Slice<const uint8_t>(
static_cast<uint8_t*>(file.file_ptr), file.file_size);
auto value_cxx = get_boolean_flag_value_cxx(content, offset);
auto value = BooleanFlagValueQuery();
value.query_success = value_cxx.query_success;
value.error_message = std::string(value_cxx.error_message.c_str());
value.flag_value = value_cxx.flag_value;
return value;
if (value_cxx.query_success) {
return value_cxx.flag_value;
} else {
return Error() << value_cxx.error_message;
}
}
} // namespace aconfig_storage

View file

@ -2,6 +2,7 @@
#include <stdint.h>
#include <string>
#include <android-base/result.h>
namespace aconfig_storage {
@ -18,73 +19,48 @@ struct MappedStorageFile {
size_t file_size;
};
/// Mapped storage file query
struct MappedStorageFileQuery {
bool query_success;
std::string error_message;
MappedStorageFile mapped_file;
};
/// DO NOT USE APIS IN THE FOLLOWING NAMESPACE DIRECTLY
namespace private_internal_api {
MappedStorageFileQuery get_mapped_file_impl(
std::string const& pb_file,
std::string const& container,
StorageFileType file_type);
} // namespace private_internal_api
/// Storage version number query result
struct VersionNumberQuery {
bool query_success;
std::string error_message;
uint32_t version_number;
};
/// Package offset query result
struct PackageOffsetQuery {
bool query_success;
std::string error_message;
struct PackageOffset {
bool package_exists;
uint32_t package_id;
uint32_t boolean_offset;
};
/// Flag offset query result
struct FlagOffsetQuery {
bool query_success;
std::string error_message;
struct FlagOffset {
bool flag_exists;
uint16_t flag_offset;
};
/// Boolean flag value query result
struct BooleanFlagValueQuery {
bool query_success;
std::string error_message;
bool flag_value;
};
/// DO NOT USE APIS IN THE FOLLOWING NAMESPACE DIRECTLY
namespace private_internal_api {
android::base::Result<MappedStorageFile> get_mapped_file_impl(
std::string const& pb_file,
std::string const& container,
StorageFileType file_type);
} // namespace private_internal_api
/// Get mapped storage file
/// \input container: stoarge container name
/// \input file_type: storage file type enum
/// \returns a MappedStorageFileQuery
MappedStorageFileQuery get_mapped_file(
android::base::Result<MappedStorageFile> get_mapped_file(
std::string const& container,
StorageFileType file_type);
/// Get storage file version number
/// \input file_path: the path to the storage file
/// \returns a VersionNumberQuery
VersionNumberQuery get_storage_file_version(
/// \returns the storage file version
android::base::Result<uint32_t> get_storage_file_version(
std::string const& file_path);
/// Get package offset
/// \input file: mapped storage file
/// \input package: the flag package name
/// \returns a PackageOffsetQuery
PackageOffsetQuery get_package_offset(
/// \returns a package offset
android::base::Result<PackageOffset> get_package_offset(
MappedStorageFile const& file,
std::string const& package);
@ -92,8 +68,8 @@ PackageOffsetQuery get_package_offset(
/// \input file: mapped storage file
/// \input package_id: the flag package id obtained from package offset query
/// \input flag_name: flag name
/// \returns a FlagOffsetQuery
FlagOffsetQuery get_flag_offset(
/// \returns the flag offset
android::base::Result<FlagOffset> get_flag_offset(
MappedStorageFile const& file,
uint32_t package_id,
std::string const& flag_name);
@ -101,8 +77,8 @@ FlagOffsetQuery get_flag_offset(
/// Get boolean flag value
/// \input file: mapped storage file
/// \input offset: the boolean flag value byte offset in the file
/// \returns a BooleanFlagValueQuery
BooleanFlagValueQuery get_boolean_flag_value(
/// \returns the boolean flag value
android::base::Result<bool> get_boolean_flag_value(
MappedStorageFile const& file,
uint32_t offset);

View file

@ -65,7 +65,7 @@ pub fn find_flag_offset(
#[cfg(test)]
mod tests {
use super::*;
use aconfig_storage_file::{StorageFileType, FlagTable};
use aconfig_storage_file::{FlagTable, StorageFileType};
// create test baseline, syntactic sugar
fn new_expected_node(

View file

@ -48,7 +48,7 @@ pub fn find_boolean_flag_value(buf: &[u8], flag_offset: u32) -> Result<bool, Aco
#[cfg(test)]
mod tests {
use super::*;
use aconfig_storage_file::{StorageFileType, FlagValueList};
use aconfig_storage_file::{FlagValueList, StorageFileType};
pub fn create_test_flag_value_list() -> FlagValueList {
let header = FlagValueHeader {

View file

@ -183,14 +183,9 @@ mod ffi {
pub fn get_package_offset_cxx(file: &[u8], package: &str) -> PackageOffsetQueryCXX;
pub fn get_flag_offset_cxx(
file: &[u8],
package_id: u32,
flag: &str,
) -> FlagOffsetQueryCXX;
pub fn get_flag_offset_cxx(file: &[u8], package_id: u32, flag: &str) -> FlagOffsetQueryCXX;
pub fn get_boolean_flag_value_cxx(file: &[u8], offset: u32)
-> BooleanFlagValueQueryCXX;
pub fn get_boolean_flag_value_cxx(file: &[u8], offset: u32) -> BooleanFlagValueQueryCXX;
}
}
@ -294,11 +289,7 @@ pub fn get_package_offset_cxx(file: &[u8], package: &str) -> ffi::PackageOffsetQ
}
/// Get flag start offset cc interlop
pub fn get_flag_offset_cxx(
file: &[u8],
package_id: u32,
flag: &str,
) -> ffi::FlagOffsetQueryCXX {
pub fn get_flag_offset_cxx(file: &[u8], package_id: u32, flag: &str) -> ffi::FlagOffsetQueryCXX {
ffi::FlagOffsetQueryCXX::new(find_flag_offset(file, package_id, flag))
}

View file

@ -72,7 +72,7 @@ pub fn find_package_offset(
#[cfg(test)]
mod tests {
use super::*;
use aconfig_storage_file::{StorageFileType, PackageTable};
use aconfig_storage_file::{PackageTable, StorageFileType};
pub fn create_test_package_table() -> PackageTable {
let header = PackageTableHeader {

View file

@ -93,105 +93,92 @@ class AconfigStorageTest : public ::testing::Test {
/// Test to lock down storage file version query api
TEST_F(AconfigStorageTest, test_storage_version_query) {
auto query = api::get_storage_file_version(package_map);
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_EQ(query.version_number, 1);
query = api::get_storage_file_version(flag_map);
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_EQ(query.version_number, 1);
query = api::get_storage_file_version(flag_val);
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_EQ(query.version_number, 1);
auto version = api::get_storage_file_version(package_map);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
version = api::get_storage_file_version(flag_map);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
version = api::get_storage_file_version(flag_val);
ASSERT_TRUE(version.ok());
ASSERT_EQ(*version, 1);
}
/// 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_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "vendor", api::StorageFileType::package_map);
ASSERT_FALSE(mapped_file_query.query_success);
ASSERT_EQ(mapped_file_query.error_message,
ASSERT_FALSE(mapped_file.ok());
ASSERT_EQ(mapped_file.error().message(),
"Unable to find storage files for container vendor");
}
/// Negative test to lock down the error when mapping a writeable storage file
TEST_F(AconfigStorageTest, test_writable_storage_file_mapping) {
ASSERT_TRUE(chmod(package_map.c_str(),
S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH) != -1);
auto mapped_file_query = private_api::get_mapped_file_impl(
ASSERT_TRUE(chmod(package_map.c_str(), 0666) != -1);
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::package_map);
ASSERT_FALSE(mapped_file_query.query_success);
ASSERT_EQ(mapped_file_query.error_message, "cannot map writeable file");
ASSERT_FALSE(mapped_file.ok());
ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");
ASSERT_TRUE(chmod(flag_map.c_str(),
S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH) != -1);
mapped_file_query = private_api::get_mapped_file_impl(
ASSERT_TRUE(chmod(flag_map.c_str(), 0666) != -1);
mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_map);
ASSERT_FALSE(mapped_file_query.query_success);
ASSERT_EQ(mapped_file_query.error_message, "cannot map writeable file");
ASSERT_FALSE(mapped_file.ok());
ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");
ASSERT_TRUE(chmod(flag_val.c_str(),
S_IRUSR | S_IRGRP | S_IROTH | S_IWUSR | S_IWGRP | S_IWOTH) != -1);
mapped_file_query = private_api::get_mapped_file_impl(
ASSERT_TRUE(chmod(flag_val.c_str(), 0666) != -1);
mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_val);
ASSERT_FALSE(mapped_file_query.query_success);
ASSERT_EQ(mapped_file_query.error_message, "cannot map writeable file");
ASSERT_FALSE(mapped_file.ok());
ASSERT_EQ(mapped_file.error().message(), "cannot map writeable file");
}
/// Test to lock down storage package offset query api
TEST_F(AconfigStorageTest, test_package_offset_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::package_map);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
auto query = api::get_package_offset(
mapped_file, "com.android.aconfig.storage.test_1");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_TRUE(query.package_exists);
ASSERT_EQ(query.package_id, 0);
ASSERT_EQ(query.boolean_offset, 0);
auto offset = api::get_package_offset(
*mapped_file, "com.android.aconfig.storage.test_1");
ASSERT_TRUE(offset.ok());
ASSERT_TRUE(offset->package_exists);
ASSERT_EQ(offset->package_id, 0);
ASSERT_EQ(offset->boolean_offset, 0);
query = api::get_package_offset(
mapped_file, "com.android.aconfig.storage.test_2");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_TRUE(query.package_exists);
ASSERT_EQ(query.package_id, 1);
ASSERT_EQ(query.boolean_offset, 3);
offset = api::get_package_offset(
*mapped_file, "com.android.aconfig.storage.test_2");
ASSERT_TRUE(offset.ok());
ASSERT_TRUE(offset->package_exists);
ASSERT_EQ(offset->package_id, 1);
ASSERT_EQ(offset->boolean_offset, 3);
query = api::get_package_offset(
mapped_file, "com.android.aconfig.storage.test_4");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_TRUE(query.package_exists);
ASSERT_EQ(query.package_id, 2);
ASSERT_EQ(query.boolean_offset, 6);
offset = api::get_package_offset(
*mapped_file, "com.android.aconfig.storage.test_4");
ASSERT_TRUE(offset.ok());
ASSERT_TRUE(offset->package_exists);
ASSERT_EQ(offset->package_id, 2);
ASSERT_EQ(offset->boolean_offset, 6);
}
/// Test to lock down when querying none exist package
TEST_F(AconfigStorageTest, test_none_existent_package_offset_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::package_map);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
auto query = api::get_package_offset(
mapped_file, "com.android.aconfig.storage.test_3");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_FALSE(query.package_exists);
auto offset = api::get_package_offset(
*mapped_file, "com.android.aconfig.storage.test_3");
ASSERT_TRUE(offset.ok());
ASSERT_FALSE(offset->package_exists);
}
/// Test to lock down storage flag offset query api
TEST_F(AconfigStorageTest, test_flag_offset_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_map);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
auto baseline = std::vector<std::tuple<int, std::string, int>>{
{0, "enabled_ro", 1},
@ -204,56 +191,49 @@ TEST_F(AconfigStorageTest, test_flag_offset_query) {
{0, "disabled_rw", 0},
};
for (auto const&[package_id, flag_name, expected_offset] : baseline) {
auto query = api::get_flag_offset(mapped_file, package_id, flag_name);
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_TRUE(query.flag_exists);
ASSERT_EQ(query.flag_offset, expected_offset);
auto offset = api::get_flag_offset(*mapped_file, package_id, flag_name);
ASSERT_TRUE(offset.ok());
ASSERT_TRUE(offset->flag_exists);
ASSERT_EQ(offset->flag_offset, expected_offset);
}
}
/// Test to lock down when querying none exist flag
TEST_F(AconfigStorageTest, test_none_existent_flag_offset_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_map);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
auto query = api::get_flag_offset(mapped_file, 0, "none_exist");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_FALSE(query.flag_exists);
auto offset = api::get_flag_offset(*mapped_file, 0, "none_exist");
ASSERT_TRUE(offset.ok());
ASSERT_FALSE(offset->flag_exists);
query = api::get_flag_offset(mapped_file, 3, "enabled_ro");
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_FALSE(query.flag_exists);
offset = api::get_flag_offset(*mapped_file, 3, "enabled_ro");
ASSERT_TRUE(offset.ok());
ASSERT_FALSE(offset->flag_exists);
}
/// Test to lock down storage flag value query api
TEST_F(AconfigStorageTest, test_boolean_flag_value_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_val);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
for (int offset = 0; offset < 8; ++offset) {
auto query = api::get_boolean_flag_value(mapped_file, offset);
ASSERT_EQ(query.error_message, std::string());
ASSERT_TRUE(query.query_success);
ASSERT_FALSE(query.flag_value);
auto value = api::get_boolean_flag_value(*mapped_file, offset);
ASSERT_TRUE(value.ok());
ASSERT_FALSE(*value);
}
}
/// Negative test to lock down the error when querying flag value out of range
TEST_F(AconfigStorageTest, test_invalid_boolean_flag_value_query) {
auto mapped_file_query = private_api::get_mapped_file_impl(
auto mapped_file = private_api::get_mapped_file_impl(
storage_record_pb, "system", api::StorageFileType::flag_val);
ASSERT_TRUE(mapped_file_query.query_success);
auto mapped_file = mapped_file_query.mapped_file;
ASSERT_TRUE(mapped_file.ok());
auto query = api::get_boolean_flag_value(mapped_file, 8);
ASSERT_EQ(query.error_message,
auto value = api::get_boolean_flag_value(*mapped_file, 8);
ASSERT_FALSE(value.ok());
ASSERT_EQ(value.error().message(),
std::string("InvalidStorageFileOffset(Flag value offset goes beyond the end of the file.)"));
ASSERT_FALSE(query.query_success);
}

View file

@ -114,9 +114,7 @@ files {{
];
for (package_id, flag_name, expected_offset) in baseline.into_iter() {
let flag_offset =
get_flag_offset(&flag_mapped_file, package_id, flag_name)
.unwrap()
.unwrap();
get_flag_offset(&flag_mapped_file, package_id, flag_name).unwrap().unwrap();
assert_eq!(flag_offset, expected_offset);
}
}
@ -128,12 +126,10 @@ files {{
let flag_mapped_file =
get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap();
let flag_offset_option =
get_flag_offset(&flag_mapped_file, 0, "none_exist").unwrap();
let flag_offset_option = get_flag_offset(&flag_mapped_file, 0, "none_exist").unwrap();
assert_eq!(flag_offset_option, None);
let flag_offset_option =
get_flag_offset(&flag_mapped_file, 3, "enabled_ro").unwrap();
let flag_offset_option = get_flag_offset(&flag_mapped_file, 3, "enabled_ro").unwrap();
assert_eq!(flag_offset_option, None);
}
@ -146,8 +142,7 @@ files {{
let baseline: Vec<bool> = vec![false; 8];
for (offset, expected_value) in baseline.into_iter().enumerate() {
let flag_value =
get_boolean_flag_value(&flag_value_file, offset as u32).unwrap();
let flag_value = get_boolean_flag_value(&flag_value_file, offset as u32).unwrap();
assert_eq!(flag_value, expected_value);
}
}

View file

@ -115,9 +115,9 @@ TEST_F(AconfigStorageTest, test_boolean_flag_value_update) {
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 value_query = api::get_boolean_flag_value(ro_mapped_file, offset);
ASSERT_TRUE(value_query.query_success);
ASSERT_TRUE(value_query.flag_value);
auto value = api::get_boolean_flag_value(ro_mapped_file, offset);
ASSERT_TRUE(value.ok());
ASSERT_TRUE(*value);
}
}