Merge "Prune unnecessary symbols from read API" into main
diff --git a/tools/aconfig/aconfig/src/commands.rs b/tools/aconfig/aconfig/src/commands.rs
index 9cb40bf..ad96bb8 100644
--- a/tools/aconfig/aconfig/src/commands.rs
+++ b/tools/aconfig/aconfig/src/commands.rs
@@ -239,10 +239,8 @@
container: &str,
file: &StorageFileType,
) -> Result<Vec<u8>> {
- let parsed_flags_vec: Vec<ProtoParsedFlags> = caches
- .into_iter()
- .map(|mut input| input.try_parse_flags())
- .collect::<Result<Vec<_>>>()?;
+ let parsed_flags_vec: Vec<ProtoParsedFlags> =
+ caches.into_iter().map(|mut input| input.try_parse_flags()).collect::<Result<Vec<_>>>()?;
generate_storage_file(container, parsed_flags_vec.iter(), file)
}
diff --git a/tools/aconfig/aconfig/src/storage/mod.rs b/tools/aconfig/aconfig/src/storage/mod.rs
index 855ed02..73339f2 100644
--- a/tools/aconfig/aconfig/src/storage/mod.rs
+++ b/tools/aconfig/aconfig/src/storage/mod.rs
@@ -189,14 +189,14 @@
assert_eq!(packages[1].package_id, 1);
assert_eq!(packages[1].flag_names.len(), 3);
assert!(packages[1].flag_names.contains("enabled_ro"));
- assert!(packages[1].flag_names.contains("disabled_ro"));
+ assert!(packages[1].flag_names.contains("disabled_rw"));
assert!(packages[1].flag_names.contains("enabled_fixed_ro"));
assert_eq!(packages[1].boolean_start_index, 3);
assert_eq!(packages[2].package_name, "com.android.aconfig.storage.test_4");
assert_eq!(packages[2].package_id, 2);
assert_eq!(packages[2].flag_names.len(), 2);
- assert!(packages[2].flag_names.contains("enabled_ro"));
+ assert!(packages[2].flag_names.contains("enabled_rw"));
assert!(packages[2].flag_names.contains("enabled_fixed_ro"));
assert_eq!(packages[2].boolean_start_index, 6);
}
diff --git a/tools/aconfig/aconfig/tests/storage_test_2.aconfig b/tools/aconfig/aconfig/tests/storage_test_2.aconfig
index bb14fd1..db77f7a 100644
--- a/tools/aconfig/aconfig/tests/storage_test_2.aconfig
+++ b/tools/aconfig/aconfig/tests/storage_test_2.aconfig
@@ -9,7 +9,7 @@
}
flag {
- name: "disabled_ro"
+ name: "disabled_rw"
namespace: "aconfig_test"
description: "This flag is DISABLED + READ_ONLY"
bug: "123"
diff --git a/tools/aconfig/aconfig/tests/storage_test_2.values b/tools/aconfig/aconfig/tests/storage_test_2.values
index a7bb0b1..b650721 100644
--- a/tools/aconfig/aconfig/tests/storage_test_2.values
+++ b/tools/aconfig/aconfig/tests/storage_test_2.values
@@ -6,9 +6,9 @@
}
flag_value {
package: "com.android.aconfig.storage.test_2"
- name: "disabled_ro"
+ name: "disabled_rw"
state: DISABLED
- permission: READ_ONLY
+ permission: READ_WRITE
}
flag_value {
package: "com.android.aconfig.storage.test_2"
diff --git a/tools/aconfig/aconfig/tests/storage_test_4.aconfig b/tools/aconfig/aconfig/tests/storage_test_4.aconfig
index 333fe09..5802a73 100644
--- a/tools/aconfig/aconfig/tests/storage_test_4.aconfig
+++ b/tools/aconfig/aconfig/tests/storage_test_4.aconfig
@@ -2,7 +2,7 @@
container: "system"
flag {
- name: "enabled_ro"
+ name: "enabled_rw"
namespace: "aconfig_test"
description: "This flag is ENABLED + READ_ONLY"
bug: "abc"
diff --git a/tools/aconfig/aconfig/tests/storage_test_4.values b/tools/aconfig/aconfig/tests/storage_test_4.values
index fa21317..784b744 100644
--- a/tools/aconfig/aconfig/tests/storage_test_4.values
+++ b/tools/aconfig/aconfig/tests/storage_test_4.values
@@ -1,8 +1,8 @@
flag_value {
package: "com.android.aconfig.storage.test_4"
- name: "enabled_ro"
+ name: "enabled_rw"
state: ENABLED
- permission: READ_ONLY
+ permission: READ_WRITE
}
flag_value {
package: "com.android.aconfig.storage.test_4"
diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs
index 070a3cf..2acfc7d 100644
--- a/tools/aconfig/aconfig_storage_file/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_file/src/lib.rs
@@ -357,8 +357,8 @@
),
(
String::from("com.android.aconfig.storage.test_2"),
- String::from("disabled_ro"),
- StoredFlagType::ReadOnlyBoolean,
+ String::from("disabled_rw"),
+ StoredFlagType::ReadWriteBoolean,
false,
),
(
@@ -381,8 +381,8 @@
),
(
String::from("com.android.aconfig.storage.test_4"),
- String::from("enabled_ro"),
- StoredFlagType::ReadOnlyBoolean,
+ String::from("enabled_rw"),
+ StoredFlagType::ReadWriteBoolean,
true,
),
];
diff --git a/tools/aconfig/aconfig_storage_file/src/test_utils.rs b/tools/aconfig/aconfig_storage_file/src/test_utils.rs
index 608563c..106666c 100644
--- a/tools/aconfig/aconfig_storage_file/src/test_utils.rs
+++ b/tools/aconfig/aconfig_storage_file/src/test_utils.rs
@@ -92,8 +92,8 @@
None,
None,
None,
- Some(178),
None,
+ Some(177),
Some(204),
None,
Some(262),
@@ -108,8 +108,8 @@
let nodes = vec![
FlagTableNode::new_expected(0, "enabled_ro", 1, 1, None),
FlagTableNode::new_expected(0, "enabled_rw", 0, 2, Some(151)),
- FlagTableNode::new_expected(1, "disabled_ro", 1, 0, None),
- FlagTableNode::new_expected(2, "enabled_ro", 1, 1, None),
+ FlagTableNode::new_expected(2, "enabled_rw", 0, 1, None),
+ FlagTableNode::new_expected(1, "disabled_rw", 0, 0, None),
FlagTableNode::new_expected(1, "enabled_fixed_ro", 2, 1, Some(236)),
FlagTableNode::new_expected(1, "enabled_ro", 1, 2, None),
FlagTableNode::new_expected(2, "enabled_fixed_ro", 2, 0, None),
@@ -140,7 +140,7 @@
num_flags: 8,
boolean_flag_offset: 27,
};
- let is_flag_rw = [true, false, true, false, false, false, false, false];
+ let is_flag_rw = [true, false, true, true, false, false, false, true];
let nodes = is_flag_rw.iter().map(|&rw| FlagInfoNode::create(rw)).collect();
FlagInfoList { header, nodes }
}
diff --git a/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp b/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp
index ff2f38e..8fe42ce 100644
--- a/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp
+++ b/tools/aconfig/aconfig_storage_read_api/aconfig_storage_read_api.cpp
@@ -65,8 +65,24 @@
return Error() << "Unable to find storage files for container " << container;;
}
+namespace private_internal_api {
+
+/// Get mapped file implementation.
+Result<MappedStorageFile> get_mapped_file_impl(
+ std::string const& pb_file,
+ std::string const& container,
+ StorageFileType file_type) {
+ auto file_result = find_storage_file(pb_file, container, file_type);
+ if (!file_result.ok()) {
+ return Error() << file_result.error();
+ }
+ return map_storage_file(*file_result);
+}
+
+} // namespace private internal api
+
/// Map a storage file
-static Result<MappedStorageFile> map_storage_file(std::string const& file) {
+Result<MappedStorageFile> map_storage_file(std::string const& file) {
int fd = open(file.c_str(), O_CLOEXEC | O_NOFOLLOW | O_RDONLY);
if (fd == -1) {
return ErrnoError() << "failed to open " << file;
@@ -90,22 +106,6 @@
return mapped_file;
}
-namespace private_internal_api {
-
-/// Get mapped file implementation.
-Result<MappedStorageFile> get_mapped_file_impl(
- std::string const& pb_file,
- std::string const& container,
- StorageFileType file_type) {
- auto file_result = find_storage_file(pb_file, container, file_type);
- if (!file_result.ok()) {
- return Error() << file_result.error();
- }
- return map_storage_file(*file_result);
-}
-
-} // namespace private internal api
-
/// Map from StoredFlagType to FlagValueType
android::base::Result<FlagValueType> map_to_flag_value_type(
StoredFlagType stored_type) {
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 0a4d5b4..2bd84fc 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
@@ -67,6 +67,11 @@
} // namespace private_internal_api
+/// Map a storage file
+android::base::Result<MappedStorageFile> map_storage_file(
+ std::string const& file);
+
+
/// Map from StoredFlagType to FlagValueType
/// \input stored_type: stored flag type in the storage file
/// \returns the flag value type enum
diff --git a/tools/aconfig/aconfig_storage_read_api/src/flag_info_query.rs b/tools/aconfig/aconfig_storage_read_api/src/flag_info_query.rs
index 0087feb..6d03377 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/flag_info_query.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/flag_info_query.rs
@@ -70,7 +70,7 @@
// this test point locks down query if flag is readwrite
fn test_is_flag_readwrite() {
let flag_info_list = create_test_flag_info_list().into_bytes();
- let baseline: Vec<bool> = vec![true, false, true, false, false, false, false, false];
+ let baseline: Vec<bool> = vec![true, false, true, true, false, false, false, true];
for offset in 0..8 {
let attribute =
find_flag_attribute(&flag_info_list[..], FlagValueType::Boolean, offset).unwrap();
diff --git a/tools/aconfig/aconfig_storage_read_api/src/flag_table_query.rs b/tools/aconfig/aconfig_storage_read_api/src/flag_table_query.rs
index 55fdcb7..a1a4793 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/flag_table_query.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/flag_table_query.rs
@@ -82,8 +82,8 @@
let baseline = vec![
(0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
(0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
- (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
- (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+ (2, "enabled_rw", StoredFlagType::ReadWriteBoolean, 1u16),
+ (1, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
(1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
(1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
(2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
diff --git a/tools/aconfig/aconfig_storage_read_api/src/lib.rs b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
index fa8dad6..e65dcfb 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
@@ -468,8 +468,8 @@
let baseline = vec![
(0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
(0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
- (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
- (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+ (2, "enabled_rw", StoredFlagType::ReadWriteBoolean, 1u16),
+ (1, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
(1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
(1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
(2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/flag.map b/tools/aconfig/aconfig_storage_read_api/tests/flag.map
index d26e00f..e868f53 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/flag.map
+++ b/tools/aconfig/aconfig_storage_read_api/tests/flag.map
Binary files differ
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp
index 5b4d32c..b499c1c 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp
+++ b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.cpp
@@ -167,8 +167,8 @@
auto baseline = std::vector<std::tuple<int, std::string, api::StoredFlagType, int>>{
{0, "enabled_ro", api::StoredFlagType::ReadOnlyBoolean, 1},
{0, "enabled_rw", api::StoredFlagType::ReadWriteBoolean, 2},
- {1, "disabled_ro", api::StoredFlagType::ReadOnlyBoolean, 0},
- {2, "enabled_ro", api::StoredFlagType::ReadOnlyBoolean, 1},
+ {2, "enabled_rw", api::StoredFlagType::ReadWriteBoolean, 1},
+ {1, "disabled_rw", api::StoredFlagType::ReadWriteBoolean, 0},
{1, "enabled_fixed_ro", api::StoredFlagType::FixedReadOnlyBoolean, 1},
{1, "enabled_ro", api::StoredFlagType::ReadOnlyBoolean, 2},
{2, "enabled_fixed_ro", api::StoredFlagType::FixedReadOnlyBoolean, 0},
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs
index 1a8af75..ce9c018 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs
+++ b/tools/aconfig/aconfig_storage_read_api/tests/storage_read_api_test.rs
@@ -118,8 +118,8 @@
let baseline = vec![
(0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
(0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
- (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
- (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+ (2, "enabled_rw", StoredFlagType::ReadWriteBoolean, 1u16),
+ (1, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
(1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
(1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
(2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
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 f031d01..cd57f4f 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
@@ -17,78 +17,6 @@
namespace aconfig_storage {
-/// Storage location pb file
-static constexpr char kPersistStorageRecordsPb[] =
- "/metadata/aconfig/persistent_storage_file_records.pb";
-
-/// Read aconfig storage records pb file
-static Result<storage_records_pb> read_storage_records_pb(std::string const& pb_file) {
- auto records = storage_records_pb();
- auto content = std::string();
- if (!ReadFileToString(pb_file, &content)) {
- return ErrnoError() << "ReadFileToString failed";
- }
-
- if (!records.ParseFromString(content)) {
- return ErrnoError() << "Unable to parse persistent storage records protobuf";
- }
- return records;
-}
-
-/// Get storage file path
-static Result<std::string> find_storage_file(
- std::string const& pb_file,
- 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
- << " : " << records_pb.error();
- }
-
- for (auto& entry : records_pb->files()) {
- if (entry.container() == container) {
- 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;
-}
-
-
-namespace private_internal_api {
-
-/// Get mutable mapped file implementation.
-Result<MutableMappedStorageFile> get_mutable_mapped_file_impl(
- std::string const& pb_file,
- 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();
- }
-
- return map_mutable_storage_file(*file_result);
-}
-
-} // namespace private internal api
-
/// Map a storage file
Result<MutableMappedStorageFile> map_mutable_storage_file(std::string const& file) {
struct stat file_stat;
@@ -120,14 +48,6 @@
return mapped_file;
}
-/// Get mutable mapped file
-Result<MutableMappedStorageFile> 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<void> set_boolean_flag_value(
const MutableMappedStorageFile& file,
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 12b0ecc..7148396 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
@@ -16,25 +16,10 @@
size_t file_size;
};
-/// DO NOT USE APIS IN THE FOLLOWING NAMESPACE DIRECTLY
-namespace private_internal_api {
-
-Result<MutableMappedStorageFile> get_mutable_mapped_file_impl(
- std::string const& pb_file,
- std::string const& container,
- StorageFileType file_type);
-
-} // namespace private_internal_api
-
/// Map a storage file
Result<MutableMappedStorageFile> map_mutable_storage_file(
std::string const& file);
-/// Get mapped writeable storage file
-Result<MutableMappedStorageFile> get_mutable_mapped_file(
- std::string const& container,
- StorageFileType file_type);
-
/// Set boolean flag value
Result<void> set_boolean_flag_value(
const MutableMappedStorageFile& file,
diff --git a/tools/aconfig/aconfig_storage_write_api/src/lib.rs b/tools/aconfig/aconfig_storage_write_api/src/lib.rs
index bcddce1..aec28de 100644
--- a/tools/aconfig/aconfig_storage_write_api/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_write_api/src/lib.rs
@@ -34,15 +34,9 @@
use std::fs::File;
use std::io::{Read, Write};
-/// Storage file location pb file
-pub const STORAGE_LOCATION_FILE: &str = "/metadata/aconfig/persistent_storage_file_records.pb";
-
/// Get read write mapped storage files.
///
-/// \input container: the flag package container
-/// \input file_type: storage file type enum
-/// \return a result of read write mapped file
-///
+/// \input file_path: path to the storage file
///
/// # Safety
///
@@ -50,11 +44,8 @@
/// file not thru this memory mapped file or there are concurrent writes to this
/// memory mapped file. Ensure all writes to the underlying file are thru this memory
/// mapped file and there are no concurrent writes.
-pub unsafe fn get_mapped_storage_file(
- container: &str,
- file_type: StorageFileType,
-) -> Result<MmapMut, AconfigStorageError> {
- unsafe { crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type) }
+pub unsafe fn map_mutable_storage_file(file_path: &str) -> Result<MmapMut, AconfigStorageError> {
+ crate::mapped_file::map_file(file_path)
}
/// Set boolean flag value thru mapped file and flush the change to file
@@ -344,7 +335,6 @@
mod tests {
use super::*;
use crate::test_utils::copy_to_temp_file;
- use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
use aconfig_storage_file::test_utils::{
create_test_flag_info_list, create_test_flag_table, create_test_package_table,
write_bytes_to_temp_file,
@@ -367,33 +357,12 @@
fn test_set_boolean_flag_value() {
let flag_value_file = copy_to_temp_file("./tests/flag.val", false).unwrap();
let flag_value_path = flag_value_file.path().display().to_string();
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "{}"
- flag_info: "some_flag.info"
- timestamp: 12345
-}}
-"#,
- flag_value_path
- );
- let record_pb_file = write_proto_to_temp_file(&text_proto).unwrap();
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is guaranteed as only this single threaded test process will
// write to this file
unsafe {
- let mut file = crate::mapped_file::get_mapped_file(
- &record_pb_path,
- "system",
- StorageFileType::FlagVal,
- )
- .unwrap();
+ let mut file = map_mutable_storage_file(&flag_value_path).unwrap();
for i in 0..8 {
set_boolean_flag_value(&mut file, i, true).unwrap();
let value = get_boolean_flag_value_at_offset(&flag_value_path, i);
@@ -417,33 +386,12 @@
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!(
- r#"
- files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "some_flag.val"
- flag_info: "{}"
- timestamp: 12345
- }}
- "#,
- flag_info_path
- );
- let record_pb_file = write_proto_to_temp_file(&text_proto).unwrap();
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is guaranteed as only this single threaded test process will
// write to this file
unsafe {
- let mut file = crate::mapped_file::get_mapped_file(
- &record_pb_path,
- "system",
- StorageFileType::FlagInfo,
- )
- .unwrap();
+ let mut file = map_mutable_storage_file(&flag_info_path).unwrap();
for i in 0..8 {
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
@@ -461,33 +409,12 @@
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!(
- r#"
- files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "some_flag.val"
- flag_info: "{}"
- timestamp: 12345
- }}
- "#,
- flag_info_path
- );
- let record_pb_file = write_proto_to_temp_file(&text_proto).unwrap();
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is guaranteed as only this single threaded test process will
// write to this file
unsafe {
- let mut file = crate::mapped_file::get_mapped_file(
- &record_pb_path,
- "system",
- StorageFileType::FlagInfo,
- )
- .unwrap();
+ let mut file = map_mutable_storage_file(&flag_info_path).unwrap();
for i in 0..8 {
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
diff --git a/tools/aconfig/aconfig_storage_write_api/src/mapped_file.rs b/tools/aconfig/aconfig_storage_write_api/src/mapped_file.rs
index ea9ac19..401d6b7 100644
--- a/tools/aconfig/aconfig_storage_write_api/src/mapped_file.rs
+++ b/tools/aconfig/aconfig_storage_write_api/src/mapped_file.rs
@@ -17,11 +17,8 @@
use anyhow::anyhow;
use memmap2::MmapMut;
use std::fs::{self, OpenOptions};
-use std::io::Read;
use aconfig_storage_file::AconfigStorageError::{self, FileReadFail, MapFileFail};
-use aconfig_storage_file::StorageFileType;
-use aconfig_storage_read_api::mapped_file::find_container_storage_location;
/// Get the mutable memory mapping of a storage file
///
@@ -31,7 +28,7 @@
/// file not thru this memory mapped file or there are concurrent writes to this
/// memory mapped file. Ensure all writes to the underlying file are thru this memory
/// mapped file and there are no concurrent writes.
-unsafe fn map_file(file_path: &str) -> Result<MmapMut, AconfigStorageError> {
+pub(crate) unsafe fn map_file(file_path: &str) -> Result<MmapMut, AconfigStorageError> {
// make sure file has read write permission
let perms = fs::metadata(file_path).unwrap().permissions();
if perms.readonly() {
@@ -51,57 +48,18 @@
}
}
-/// Get a mapped storage file given the container and file type
-///
-/// # Safety
-///
-/// The memory mapped file may have undefined behavior if there are writes to this
-/// file not thru this memory mapped file or there are concurrent writes to this
-/// memory mapped file. Ensure all writes to the underlying file are thru this memory
-/// mapped file and there are no concurrent writes.
-pub unsafe fn get_mapped_file(
- location_pb_file: &str,
- container: &str,
- file_type: StorageFileType,
-) -> Result<MmapMut, AconfigStorageError> {
- let files_location = find_container_storage_location(location_pb_file, container)?;
- match file_type {
- StorageFileType::FlagVal => unsafe { map_file(files_location.flag_val()) },
- StorageFileType::FlagInfo => unsafe { map_file(files_location.flag_info()) },
- _ => Err(MapFileFail(anyhow!(
- "Cannot map file type {:?} as writeable memory mapped files.",
- file_type
- ))),
- }
-}
-
#[cfg(test)]
mod tests {
use super::*;
use crate::test_utils::copy_to_temp_file;
- use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
+ use std::io::Read;
#[test]
fn test_mapped_file_contents() {
let mut rw_val_file = copy_to_temp_file("./tests/flag.val", false).unwrap();
let mut rw_info_file = copy_to_temp_file("./tests/flag.info", false).unwrap();
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "{}"
- flag_info: "{}"
- timestamp: 12345
-}}
-"#,
- rw_val_file.path().display().to_string(),
- rw_info_file.path().display().to_string()
- );
- let storage_record_file = write_proto_to_temp_file(&text_proto).unwrap();
- let storage_record_file_path = storage_record_file.path().display().to_string();
+ let flag_val = rw_val_file.path().display().to_string();
+ let flag_info = rw_info_file.path().display().to_string();
let mut content = Vec::new();
rw_val_file.read_to_end(&mut content).unwrap();
@@ -109,9 +67,7 @@
// SAFETY:
// The safety here is guaranteed here as no writes happens to this temp file
unsafe {
- let mmaped_file =
- get_mapped_file(&storage_record_file_path, "system", StorageFileType::FlagVal)
- .unwrap();
+ let mmaped_file = map_file(&flag_val).unwrap();
assert_eq!(mmaped_file[..], content[..]);
}
@@ -121,90 +77,23 @@
// SAFETY:
// The safety here is guaranteed here as no writes happens to this temp file
unsafe {
- let mmaped_file =
- get_mapped_file(&storage_record_file_path, "system", StorageFileType::FlagInfo)
- .unwrap();
+ let mmaped_file = map_file(&flag_info).unwrap();
assert_eq!(mmaped_file[..], content[..]);
}
}
#[test]
fn test_mapped_read_only_file() {
- let ro_file = copy_to_temp_file("./tests/flag.val", true).unwrap();
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "{}"
- flag_info: "some_flag.info"
- timestamp: 12345
-}}
-"#,
- ro_file.path().display().to_string()
- );
- let storage_record_file = write_proto_to_temp_file(&text_proto).unwrap();
- let storage_record_file_path = storage_record_file.path().display().to_string();
+ let ro_val_file = copy_to_temp_file("./tests/flag.val", true).unwrap();
+ let flag_val = ro_val_file.path().display().to_string();
// SAFETY:
// The safety here is guaranteed here as no writes happens to this temp file
unsafe {
- let error =
- get_mapped_file(&storage_record_file_path, "system", StorageFileType::FlagVal)
- .unwrap_err();
+ let error = map_file(&flag_val).unwrap_err();
assert_eq!(
format!("{:?}", error),
- format!(
- "MapFileFail(fail to map non read write storage file {})",
- ro_file.path().display().to_string()
- )
- );
- }
- }
-
- #[test]
- fn test_mapped_not_supported_file() {
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "some_package.map"
- flag_map: "some_flag.map"
- flag_val: "some_flag.val"
- flag_info: "some_flag.info"
- timestamp: 12345
-}}
-"#,
- );
- let storage_record_file = write_proto_to_temp_file(&text_proto).unwrap();
- let storage_record_file_path = storage_record_file.path().display().to_string();
-
- // SAFETY:
- // The safety here is guaranteed here as no writes happens to this temp file
- unsafe {
- let error =
- get_mapped_file(&storage_record_file_path, "system", StorageFileType::PackageMap)
- .unwrap_err();
- assert_eq!(
- format!("{:?}", error),
- format!(
- "MapFileFail(Cannot map file type {:?} as writeable memory mapped files.)",
- StorageFileType::PackageMap
- )
- );
-
- let error =
- get_mapped_file(&storage_record_file_path, "system", StorageFileType::FlagMap)
- .unwrap_err();
- assert_eq!(
- format!("{:?}", error),
- format!(
- "MapFileFail(Cannot map file type {:?} as writeable memory mapped files.)",
- StorageFileType::FlagMap
- )
+ format!("MapFileFail(fail to map non read write storage file {})", flag_val)
);
}
}
diff --git a/tools/aconfig/aconfig_storage_write_api/tests/flag.info b/tools/aconfig/aconfig_storage_write_api/tests/flag.info
index 820d839..6223edf 100644
--- a/tools/aconfig/aconfig_storage_write_api/tests/flag.info
+++ b/tools/aconfig/aconfig_storage_write_api/tests/flag.info
Binary files differ
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 ab5f0a7..bd39e9e 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,88 +50,36 @@
return temp_file;
}
- Result<std::string> 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();
- info->set_version(0);
- info->set_container("mockup");
- 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();
- proto.SerializeToString(&content);
- if (!WriteStringToFile(content, temp_file)) {
- return Error() << "failed to write storage records pb file";
- }
- return temp_file;
- }
-
void SetUp() override {
auto const test_dir = android::base::GetExecutableDirectory();
flag_val = *copy_to_rw_temp_file(test_dir + "/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_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");
-}
-
/// 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_mutable_mapped_file_impl(
- storage_record_pb, "mockup", api::StorageFileType::flag_val);
+ auto mapped_file_result = api::map_mutable_storage_file(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_mutable_mapped_file_impl(
- storage_record_pb, "mockup", api::StorageFileType::flag_val);
+ auto mapped_file_result = api::map_mutable_storage_file(flag_val);
ASSERT_TRUE(mapped_file_result.ok());
- auto mapped_file = *mapped_file_result;
+ auto mapped_file = *mapped_file_result;
for (int offset = 0; offset < 8; ++offset) {
auto update_result = api::set_boolean_flag_value(mapped_file, offset, true);
ASSERT_TRUE(update_result.ok());
@@ -146,10 +94,10 @@
/// 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_mutable_mapped_file_impl(
- storage_record_pb, "mockup", api::StorageFileType::flag_val);
+ auto mapped_file_result = api::map_mutable_storage_file(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);
ASSERT_FALSE(update_result.ok());
ASSERT_EQ(update_result.error().message(),
@@ -158,15 +106,14 @@
/// 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);
+ auto mapped_file_result = api::map_mutable_storage_file(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_server_override(
mapped_file, api::FlagValueType::Boolean, offset, true);
- ASSERT_TRUE(update_result.ok());
+ ASSERT_TRUE(update_result.ok()) << update_result.error();
auto ro_mapped_file = api::MappedStorageFile();
ro_mapped_file.file_ptr = mapped_file.file_ptr;
ro_mapped_file.file_size = mapped_file.file_size;
@@ -189,8 +136,7 @@
/// 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);
+ auto mapped_file_result = api::map_mutable_storage_file(flag_info);
ASSERT_TRUE(mapped_file_result.ok());
auto mapped_file = *mapped_file_result;
diff --git a/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.rs b/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.rs
index a087c1a..367569d 100644
--- a/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.rs
+++ b/tools/aconfig/aconfig_storage_write_api/tests/storage_write_api_test.rs
@@ -1,44 +1,17 @@
#[cfg(not(feature = "cargo"))]
mod aconfig_storage_write_api_test {
- use aconfig_storage_file::protos::ProtoStorageFiles;
- use aconfig_storage_file::{FlagInfoBit, FlagValueType, StorageFileType};
+ use aconfig_storage_file::{FlagInfoBit, FlagValueType};
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_local_override,
+ map_mutable_storage_file, set_boolean_flag_value, set_flag_has_local_override,
set_flag_has_server_override,
};
- use protobuf::Message;
use std::fs::{self, File};
- use std::io::{Read, Write};
+ use std::io::Read;
use tempfile::NamedTempFile;
- /// Write storage location record pb to a temp file
- fn write_storage_record_file(flag_val: &str, flag_info: &str) -> NamedTempFile {
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "mockup"
- package_map: "some_package_map"
- flag_map: "some_flag_map"
- flag_val: "{}"
- flag_info: "{}"
- timestamp: 12345
-}}
-"#,
- flag_val, flag_info
- );
- let storage_files: ProtoStorageFiles =
- protobuf::text_format::parse_from_str(&text_proto).unwrap();
- let mut binary_proto_bytes = Vec::new();
- storage_files.write_to_vec(&mut binary_proto_bytes).unwrap();
- let mut file = NamedTempFile::new().unwrap();
- file.write_all(&binary_proto_bytes).unwrap();
- file
- }
-
/// Create temp file copy
fn copy_to_temp_rw_file(source_file: &str) -> NamedTempFile {
let file = NamedTempFile::new().unwrap();
@@ -66,18 +39,12 @@
/// Test to lock down flag value update api
fn test_boolean_flag_value_update() {
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();
- let flag_info_path = flag_info_file.path().display().to_string();
- let record_pb_file = write_storage_record_file(&flag_value_path, &flag_info_path);
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is ensured as only this single threaded test process will
// write to this file
- let mut file = unsafe {
- get_mapped_file(&record_pb_path, "mockup", StorageFileType::FlagVal).unwrap()
- };
+ let mut file = unsafe { map_mutable_storage_file(&flag_value_path).unwrap() };
for i in 0..8 {
set_boolean_flag_value(&mut file, i, true).unwrap();
let value = get_boolean_flag_value_at_offset(&flag_value_path, i);
@@ -92,19 +59,13 @@
#[test]
/// 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();
let flag_info_path = flag_info_file.path().display().to_string();
- let record_pb_file = write_storage_record_file(&flag_value_path, &flag_info_path);
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is ensured as only this single threaded test process will
// write to this file
- let mut file = unsafe {
- get_mapped_file(&record_pb_path, "mockup", StorageFileType::FlagInfo).unwrap()
- };
+ let mut file = unsafe { map_mutable_storage_file(&flag_info_path).unwrap() };
for i in 0..8 {
set_flag_has_server_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =
@@ -120,19 +81,13 @@
#[test]
/// 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();
let flag_info_path = flag_info_file.path().display().to_string();
- let record_pb_file = write_storage_record_file(&flag_value_path, &flag_info_path);
- let record_pb_path = record_pb_file.path().display().to_string();
// SAFETY:
// The safety here is ensured as only this single threaded test process will
// write to this file
- let mut file = unsafe {
- get_mapped_file(&record_pb_path, "mockup", StorageFileType::FlagInfo).unwrap()
- };
+ let mut file = unsafe { map_mutable_storage_file(&flag_info_path).unwrap() };
for i in 0..8 {
set_flag_has_local_override(&mut file, FlagValueType::Boolean, i, true).unwrap();
let attribute =