aconfig: update aconfig_storage_read_api
Right now, aconfig_storage_read_api rust lib would own mapped files. So
individual flag lib does not need to. The original intent is that all
these flag lib can dynamically link against this lib. So say we have
different variants of flag lib for one flag package, there is just one
unique memory mapped file. In addition, the exposed rust apis hides the
memory mapped files. So when exporting these apis to c++/java, we don't
have to implement memory mapping in c++/java flag read api lib.
However, this design has its own downside. The biggest being that these
memory mapped files are stored in a mutable static hash map. For rust,
it must be protected by a mutex lock. The real effect on the lock is
mostly minimal as it immediately unlocks when the clone of memory
mapping is returned. But still we could get more performance out of it
in exchange of having more c++/java side implementation of mapping
files. (Note, apis that interpret mapped files are still exported by
Rust, so no c++/java implementation needed).
In this change, aconfig_storage_read_api give away its ownership of
memory mapped file in its api to get memory mapped file. The memory
mapped file is supposed to be owned by generated flag lib. By doing so,
no more mutex lock is needed. Also, since each flag lib has one flag
package, which maps to a single owning container. The flag lib can have
the memory mapped files as non mutable static variables. So no mutex
lock penalty on flag lib side.
Bug: b/321077378
Test: atest aconfig_storage_read_api.test; atest
aconfig_storage_read_api.test.rust
Change-Id: I29060bae90db6f7d296252ed73f7e312143f4b7e
diff --git a/tools/aconfig/aconfig_storage_read_api/src/lib.rs b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
index cb6a74a..87372c6 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
@@ -42,54 +42,79 @@
#[cfg(test)]
mod test_utils;
-pub use aconfig_storage_file::{
- protos::ProtoStorageFiles, read_u32_from_bytes, AconfigStorageError, StorageFileType,
- FILE_VERSION,
-};
+pub use aconfig_storage_file::{AconfigStorageError, StorageFileType};
pub use flag_table_query::FlagOffset;
pub use package_table_query::PackageOffset;
+use aconfig_storage_file::{read_u32_from_bytes, FILE_VERSION};
use flag_table_query::find_flag_offset;
use flag_value_query::find_boolean_flag_value;
-use mapped_file::get_mapped_file;
use package_table_query::find_package_offset;
use anyhow::anyhow;
+use memmap2::Mmap;
use std::fs::File;
use std::io::Read;
/// Storage file location pb file
pub const STORAGE_LOCATION_FILE: &str = "/metadata/aconfig/available_storage_file_records.pb";
-/// Get package start offset implementation
-pub fn get_package_offset_impl(
- pb_file: &str,
+/// Get read only mapped storage files.
+///
+/// \input container: the flag package container
+/// \input file_type: stoarge file type enum
+/// \return a result of read only mapped file
+pub fn get_mapped_storage_file(
container: &str,
- package: &str,
-) -> Result<Option<PackageOffset>, AconfigStorageError> {
- let mapped_file = get_mapped_file(pb_file, container, StorageFileType::PackageMap)?;
- find_package_offset(&mapped_file, package)
+ file_type: StorageFileType,
+) -> Result<Mmap, AconfigStorageError> {
+ crate::mapped_file::get_mapped_file(STORAGE_LOCATION_FILE, container, file_type)
}
-/// Get flag offset implementation
-pub fn get_flag_offset_impl(
- pb_file: &str,
- container: &str,
+/// Get package start offset for flags.
+///
+/// \input file: mapped package file
+/// \input package: package name
+///
+/// \return
+/// If a package is found, it returns Ok(Some(PackageOffset))
+/// If a package is not found, it returns Ok(None)
+/// If errors out, it returns an Err(errmsg)
+pub fn get_package_offset(
+ file: &Mmap,
+ package: &str,
+) -> Result<Option<PackageOffset>, AconfigStorageError> {
+ find_package_offset(file, package)
+}
+
+/// Get flag offset within a package given.
+///
+/// \input file: mapped flag file
+/// \input package_id: package id obtained from package mapping file
+/// \input flag: flag name
+///
+/// \return
+/// If a flag is found, it returns Ok(Some(u16))
+/// If a flag is not found, it returns Ok(None)
+/// If errors out, it returns an Err(errmsg)
+pub fn get_flag_offset(
+ file: &Mmap,
package_id: u32,
flag: &str,
) -> Result<Option<FlagOffset>, AconfigStorageError> {
- let mapped_file = get_mapped_file(pb_file, container, StorageFileType::FlagMap)?;
- find_flag_offset(&mapped_file, package_id, flag)
+ find_flag_offset(file, package_id, flag)
}
-/// Get boolean flag value implementation
-pub fn get_boolean_flag_value_impl(
- pb_file: &str,
- container: &str,
- offset: u32,
-) -> Result<bool, AconfigStorageError> {
- let mapped_file = get_mapped_file(pb_file, container, StorageFileType::FlagVal)?;
- find_boolean_flag_value(&mapped_file, offset)
+/// Get the boolean flag value.
+///
+/// \input file: mapped flag file
+/// \input offset: flag value offset
+///
+/// \return
+/// If the provide offset is valid, it returns the boolean flag value, otherwise it
+/// returns the error message.
+pub fn get_boolean_flag_value(file: &Mmap, offset: u32) -> Result<bool, AconfigStorageError> {
+ find_boolean_flag_value(file, offset)
}
/// Get storage file version number
@@ -114,48 +139,6 @@
read_u32_from_bytes(&buffer, &mut head)
}
-/// Get package start offset for flags given the container and package name.
-///
-/// This function would map the corresponding package map file if has not been mapped yet,
-/// and then look for the target package in this mapped file.
-///
-/// If a package is found, it returns Ok(Some(PackageOffset))
-/// If a package is not found, it returns Ok(None)
-/// If errors out such as no such package map file is found, it returns an Err(errmsg)
-pub fn get_package_offset(
- container: &str,
- package: &str,
-) -> Result<Option<PackageOffset>, AconfigStorageError> {
- get_package_offset_impl(STORAGE_LOCATION_FILE, container, package)
-}
-
-/// Get flag offset within a package given the container name, package id and flag name.
-///
-/// This function would map the corresponding flag map file if has not been mapped yet,
-/// and then look for the target flag in this mapped file.
-///
-/// If a flag is found, it returns Ok(Some(u16))
-/// If a flag is not found, it returns Ok(None)
-/// If errors out such as no such flag map file is found, it returns an Err(errmsg)
-pub fn get_flag_offset(
- container: &str,
- package_id: u32,
- flag: &str,
-) -> Result<Option<FlagOffset>, AconfigStorageError> {
- get_flag_offset_impl(STORAGE_LOCATION_FILE, container, package_id, flag)
-}
-
-/// Get the boolean flag value given the container name and flag global offset
-///
-/// This function would map the corresponding flag value file if has not been mapped yet,
-/// and then look for the target flag value at the specified offset.
-///
-/// If flag value file is successfully mapped and the provide offset is valid, it returns
-/// the boolean flag value, otherwise it returns the error message.
-pub fn get_boolean_flag_value(container: &str, offset: u32) -> Result<bool, AconfigStorageError> {
- get_boolean_flag_value_impl(STORAGE_LOCATION_FILE, container, offset)
-}
-
// *************************************** //
// CC INTERLOP
// *************************************** //
@@ -196,36 +179,17 @@
// Rust export to c++
extern "Rust" {
- pub fn get_package_offset_cxx_impl(
- pb_file: &str,
- container: &str,
- package: &str,
- ) -> PackageOffsetQueryCXX;
-
- pub fn get_flag_offset_cxx_impl(
- pb_file: &str,
- container: &str,
- package_id: u32,
- flag: &str,
- ) -> FlagOffsetQueryCXX;
-
- pub fn get_boolean_flag_value_cxx_impl(
- pb_file: &str,
- container: &str,
- offset: u32,
- ) -> BooleanFlagValueQueryCXX;
-
pub fn get_storage_file_version_cxx(file_path: &str) -> VersionNumberQueryCXX;
- pub fn get_package_offset_cxx(container: &str, package: &str) -> PackageOffsetQueryCXX;
+ pub fn get_package_offset_cxx(file: &[u8], package: &str) -> PackageOffsetQueryCXX;
pub fn get_flag_offset_cxx(
- container: &str,
+ file: &[u8],
package_id: u32,
flag: &str,
) -> FlagOffsetQueryCXX;
- pub fn get_boolean_flag_value_cxx(container: &str, offset: u32)
+ pub fn get_boolean_flag_value_cxx(file: &[u8], offset: u32)
-> BooleanFlagValueQueryCXX;
}
}
@@ -324,32 +288,23 @@
}
}
-/// Get package start offset impl cc interlop
-pub fn get_package_offset_cxx_impl(
- pb_file: &str,
- container: &str,
- package: &str,
-) -> ffi::PackageOffsetQueryCXX {
- ffi::PackageOffsetQueryCXX::new(get_package_offset_impl(pb_file, container, package))
+/// Get package start offset cc interlop
+pub fn get_package_offset_cxx(file: &[u8], package: &str) -> ffi::PackageOffsetQueryCXX {
+ ffi::PackageOffsetQueryCXX::new(find_package_offset(file, package))
}
-/// Get flag start offset impl cc interlop
-pub fn get_flag_offset_cxx_impl(
- pb_file: &str,
- container: &str,
+/// Get flag start offset cc interlop
+pub fn get_flag_offset_cxx(
+ file: &[u8],
package_id: u32,
flag: &str,
) -> ffi::FlagOffsetQueryCXX {
- ffi::FlagOffsetQueryCXX::new(get_flag_offset_impl(pb_file, container, package_id, flag))
+ ffi::FlagOffsetQueryCXX::new(find_flag_offset(file, package_id, flag))
}
-/// Get boolean flag value impl cc interlop
-pub fn get_boolean_flag_value_cxx_impl(
- pb_file: &str,
- container: &str,
- offset: u32,
-) -> ffi::BooleanFlagValueQueryCXX {
- ffi::BooleanFlagValueQueryCXX::new(get_boolean_flag_value_impl(pb_file, container, offset))
+/// Get boolean flag value cc interlop
+pub fn get_boolean_flag_value_cxx(file: &[u8], offset: u32) -> ffi::BooleanFlagValueQueryCXX {
+ ffi::BooleanFlagValueQueryCXX::new(find_boolean_flag_value(file, offset))
}
/// Get storage version number cc interlop
@@ -357,45 +312,19 @@
ffi::VersionNumberQueryCXX::new(get_storage_file_version(file_path))
}
-/// Get package start offset cc interlop
-pub fn get_package_offset_cxx(container: &str, package: &str) -> ffi::PackageOffsetQueryCXX {
- ffi::PackageOffsetQueryCXX::new(get_package_offset(container, package))
-}
-
-/// Get flag start offset cc interlop
-pub fn get_flag_offset_cxx(
- container: &str,
- package_id: u32,
- flag: &str,
-) -> ffi::FlagOffsetQueryCXX {
- ffi::FlagOffsetQueryCXX::new(get_flag_offset(container, package_id, flag))
-}
-
-/// Get boolean flag value cc interlop
-pub fn get_boolean_flag_value_cxx(container: &str, offset: u32) -> ffi::BooleanFlagValueQueryCXX {
- ffi::BooleanFlagValueQueryCXX::new(get_boolean_flag_value(container, offset))
-}
-
#[cfg(test)]
mod tests {
use super::*;
- use crate::test_utils::TestStorageFileSet;
+ use crate::mapped_file::get_mapped_file;
+ use crate::test_utils::copy_to_temp_file;
use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
+ use tempfile::NamedTempFile;
- fn create_test_storage_files(read_only: bool) -> TestStorageFileSet {
- TestStorageFileSet::new(
- "./tests/package.map",
- "./tests/flag.map",
- "./tests/flag.val",
- read_only,
- )
- .unwrap()
- }
+ fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] {
+ let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap();
+ let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap();
+ let flag_val = copy_to_temp_file("./tests/flag.val", read_only).unwrap();
- #[test]
- // this test point locks down flag package offset query
- fn test_package_offset_query() {
- let ro_files = create_test_storage_files(true);
let text_proto = format!(
r#"
files {{
@@ -407,38 +336,40 @@
timestamp: 12345
}}
"#,
- ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
+ package_map.path().display(),
+ flag_map.path().display(),
+ flag_val.path().display()
);
+ let pb_file = write_proto_to_temp_file(&text_proto).unwrap();
+ [package_map, flag_map, flag_val, pb_file]
+ }
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_1",
- )
- .unwrap()
- .unwrap();
+ #[test]
+ // this test point locks down flag package offset query
+ fn test_package_offset_query() {
+ let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);
+ let pb_file_path = pb_file.path().display().to_string();
+ let package_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap();
+
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_1")
+ .unwrap()
+ .unwrap();
let expected_package_offset = PackageOffset { package_id: 0, boolean_offset: 0 };
assert_eq!(package_offset, expected_package_offset);
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_2",
- )
- .unwrap()
- .unwrap();
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_2")
+ .unwrap()
+ .unwrap();
let expected_package_offset = PackageOffset { package_id: 1, boolean_offset: 3 };
assert_eq!(package_offset, expected_package_offset);
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_4",
- )
- .unwrap()
- .unwrap();
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_4")
+ .unwrap()
+ .unwrap();
let expected_package_offset = PackageOffset { package_id: 2, boolean_offset: 6 };
assert_eq!(package_offset, expected_package_offset);
}
@@ -446,23 +377,11 @@
#[test]
// this test point locks down flag offset query
fn test_flag_offset_query() {
- let ro_files = create_test_storage_files(true);
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
- );
+ let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap();
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
let baseline = vec![
(0, "enabled_ro", 1u16),
(0, "enabled_rw", 2u16),
@@ -475,9 +394,7 @@
];
for (package_id, flag_name, expected_offset) in baseline.into_iter() {
let flag_offset =
- get_flag_offset_impl(&file_full_path, "system", package_id, flag_name)
- .unwrap()
- .unwrap();
+ get_flag_offset(&flag_mapped_file, package_id, flag_name).unwrap().unwrap();
assert_eq!(flag_offset, expected_offset);
}
}
@@ -485,27 +402,13 @@
#[test]
// this test point locks down flag offset query
fn test_flag_value_query() {
- let ro_files = create_test_storage_files(true);
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
- );
-
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
+ let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_value_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap();
let baseline: Vec<bool> = vec![false; 8];
for (offset, expected_value) in baseline.into_iter().enumerate() {
- let flag_value =
- get_boolean_flag_value_impl(&file_full_path, "system", offset as u32).unwrap();
+ let flag_value = get_boolean_flag_value(&flag_value_file, offset as u32).unwrap();
assert_eq!(flag_value, expected_value);
}
}
diff --git a/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs b/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs
index 67c293c..09ecdb6 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/mapped_file.rs
@@ -14,14 +14,11 @@
* limitations under the License.
*/
-use std::collections::HashMap;
use std::fs::{self, File};
use std::io::{BufReader, Read};
-use std::sync::{Arc, Mutex};
use anyhow::anyhow;
use memmap2::Mmap;
-use once_cell::sync::Lazy;
use crate::AconfigStorageError::{
self, FileReadFail, MapFileFail, ProtobufParseFail, StorageFileNotFound,
@@ -31,20 +28,6 @@
storage_record_pb::try_from_binary_proto, ProtoStorageFileInfo, ProtoStorageFiles,
};
-/// Cache for already mapped files
-static ALL_MAPPED_FILES: Lazy<Mutex<HashMap<String, MappedStorageFileSet>>> = Lazy::new(|| {
- let mapped_files = HashMap::new();
- Mutex::new(mapped_files)
-});
-
-/// Mapped storage files for a particular container
-#[derive(Debug)]
-struct MappedStorageFileSet {
- package_map: Arc<Mmap>,
- flag_map: Arc<Mmap>,
- flag_val: Arc<Mmap>,
-}
-
/// Find where storage files are stored for a particular container
fn find_container_storage_location(
location_pb_file: &str,
@@ -101,49 +84,26 @@
}
}
-/// Map all storage files for a particular container
-fn map_container_storage_files(
- location_pb_file: &str,
- container: &str,
-) -> Result<MappedStorageFileSet, AconfigStorageError> {
- let files_location = find_container_storage_location(location_pb_file, container)?;
- let package_map = Arc::new(verify_read_only_and_map(files_location.package_map())?);
- let flag_map = Arc::new(verify_read_only_and_map(files_location.flag_map())?);
- let flag_val = Arc::new(verify_read_only_and_map(files_location.flag_val())?);
- Ok(MappedStorageFileSet { package_map, flag_map, flag_val })
-}
-
/// Get a mapped storage file given the container and file type
-pub(crate) fn get_mapped_file(
+pub fn get_mapped_file(
location_pb_file: &str,
container: &str,
- file_selection: StorageFileType,
-) -> Result<Arc<Mmap>, AconfigStorageError> {
- let mut all_mapped_files = ALL_MAPPED_FILES.lock().unwrap();
- match all_mapped_files.get(container) {
- Some(mapped_files) => Ok(match file_selection {
- StorageFileType::PackageMap => Arc::clone(&mapped_files.package_map),
- StorageFileType::FlagMap => Arc::clone(&mapped_files.flag_map),
- StorageFileType::FlagVal => Arc::clone(&mapped_files.flag_val),
- }),
- None => {
- let mapped_files = map_container_storage_files(location_pb_file, container)?;
- let file_ptr = match file_selection {
- StorageFileType::PackageMap => Arc::clone(&mapped_files.package_map),
- StorageFileType::FlagMap => Arc::clone(&mapped_files.flag_map),
- StorageFileType::FlagVal => Arc::clone(&mapped_files.flag_val),
- };
- all_mapped_files.insert(container.to_string(), mapped_files);
- Ok(file_ptr)
- }
+ file_type: StorageFileType,
+) -> Result<Mmap, AconfigStorageError> {
+ let files_location = find_container_storage_location(location_pb_file, container)?;
+ match file_type {
+ StorageFileType::PackageMap => verify_read_only_and_map(files_location.package_map()),
+ StorageFileType::FlagMap => verify_read_only_and_map(files_location.flag_map()),
+ StorageFileType::FlagVal => verify_read_only_and_map(files_location.flag_val()),
}
}
#[cfg(test)]
mod tests {
use super::*;
- use crate::test_utils::TestStorageFileSet;
+ use crate::test_utils::copy_to_temp_file;
use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
+ use tempfile::NamedTempFile;
#[test]
fn test_find_storage_file_location() {
@@ -190,125 +150,87 @@
);
}
- fn map_and_verify(location_pb_file: &str, file_selection: StorageFileType, actual_file: &str) {
+ fn map_and_verify(location_pb_file: &str, file_type: StorageFileType, actual_file: &str) {
let mut opened_file = File::open(actual_file).unwrap();
let mut content = Vec::new();
opened_file.read_to_end(&mut content).unwrap();
- let mmaped_file = get_mapped_file(location_pb_file, "system", file_selection).unwrap();
+ let mmaped_file = get_mapped_file(location_pb_file, "system", file_type).unwrap();
assert_eq!(mmaped_file[..], content[..]);
}
- fn create_test_storage_files(read_only: bool) -> TestStorageFileSet {
- TestStorageFileSet::new(
- "./tests/package.map",
- "./tests/flag.map",
- "./tests/flag.val",
- read_only,
- )
- .unwrap()
+ fn create_test_storage_files(read_only: bool) -> [NamedTempFile; 4] {
+ let package_map = copy_to_temp_file("./tests/package.map", read_only).unwrap();
+ let flag_map = copy_to_temp_file("./tests/flag.map", read_only).unwrap();
+ let flag_val = copy_to_temp_file("./tests/package.map", read_only).unwrap();
+
+ let text_proto = format!(
+ r#"
+files {{
+ version: 0
+ container: "system"
+ package_map: "{}"
+ flag_map: "{}"
+ flag_val: "{}"
+ timestamp: 12345
+}}
+"#,
+ package_map.path().display(),
+ flag_map.path().display(),
+ flag_val.path().display()
+ );
+ let pb_file = write_proto_to_temp_file(&text_proto).unwrap();
+ [package_map, flag_map, flag_val, pb_file]
}
#[test]
fn test_mapped_file_contents() {
- let ro_files = create_test_storage_files(true);
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- ro_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
+ let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(true);
+ let pb_file_path = pb_file.path().display().to_string();
+ map_and_verify(
+ &pb_file_path,
+ StorageFileType::PackageMap,
+ &package_map.path().display().to_string(),
);
-
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
- map_and_verify(&file_full_path, StorageFileType::PackageMap, &ro_files.package_map.name);
- map_and_verify(&file_full_path, StorageFileType::FlagMap, &ro_files.flag_map.name);
- map_and_verify(&file_full_path, StorageFileType::FlagVal, &ro_files.flag_val.name);
+ map_and_verify(
+ &pb_file_path,
+ StorageFileType::FlagMap,
+ &flag_map.path().display().to_string(),
+ );
+ map_and_verify(
+ &pb_file_path,
+ StorageFileType::FlagVal,
+ &flag_val.path().display().to_string(),
+ );
}
#[test]
fn test_map_non_read_only_file() {
- let ro_files = create_test_storage_files(true);
- let rw_files = create_test_storage_files(false);
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- rw_files.package_map.name, ro_files.flag_map.name, ro_files.flag_val.name
- );
-
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
- let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
+ let [package_map, flag_map, flag_val, pb_file] = create_test_storage_files(false);
+ let pb_file_path = pb_file.path().display().to_string();
+ let error =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap_err();
assert_eq!(
format!("{:?}", error),
format!(
"MapFileFail(fail to map non read only storage file {})",
- rw_files.package_map.name
+ package_map.path().display()
)
);
-
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- ro_files.package_map.name, rw_files.flag_map.name, ro_files.flag_val.name
- );
-
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
- let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
+ let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap_err();
assert_eq!(
format!("{:?}", error),
format!(
"MapFileFail(fail to map non read only storage file {})",
- rw_files.flag_map.name
+ flag_map.path().display()
)
);
-
- let text_proto = format!(
- r#"
-files {{
- version: 0
- container: "system"
- package_map: "{}"
- flag_map: "{}"
- flag_val: "{}"
- timestamp: 12345
-}}
-"#,
- ro_files.package_map.name, ro_files.flag_map.name, rw_files.flag_val.name
- );
-
- let file = write_proto_to_temp_file(&text_proto).unwrap();
- let file_full_path = file.path().display().to_string();
- let error = map_container_storage_files(&file_full_path, "system").unwrap_err();
+ let error = get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap_err();
assert_eq!(
format!("{:?}", error),
format!(
"MapFileFail(fail to map non read only storage file {})",
- rw_files.flag_val.name
+ flag_val.path().display()
)
);
}
diff --git a/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs b/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs
index 22f367f..ff72499 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/test_utils.rs
@@ -18,49 +18,15 @@
use std::fs;
use tempfile::NamedTempFile;
-fn set_file_read_only(file: &NamedTempFile) {
- let mut perms = fs::metadata(file.path()).unwrap().permissions();
- if !perms.readonly() {
+/// Create temp file copy
+pub(crate) fn copy_to_temp_file(source_file: &str, read_only: bool) -> Result<NamedTempFile> {
+ let file = NamedTempFile::new()?;
+ fs::copy(source_file, file.path())?;
+ if read_only {
+ let file_name = file.path().display().to_string();
+ let mut perms = fs::metadata(file_name).unwrap().permissions();
perms.set_readonly(true);
- fs::set_permissions(file.path(), perms).unwrap();
+ fs::set_permissions(file.path(), perms.clone()).unwrap();
}
-}
-
-#[allow(dead_code)]
-pub(crate) struct TestStorageFile {
- pub file: NamedTempFile,
- pub name: String,
-}
-
-impl TestStorageFile {
- pub(crate) fn new(source_file: &str, read_only: bool) -> Result<Self> {
- let file = NamedTempFile::new()?;
- fs::copy(source_file, file.path())?;
- if read_only {
- set_file_read_only(&file);
- }
- let name = file.path().display().to_string();
- Ok(Self { file, name })
- }
-}
-
-pub(crate) struct TestStorageFileSet {
- pub package_map: TestStorageFile,
- pub flag_map: TestStorageFile,
- pub flag_val: TestStorageFile,
-}
-
-impl TestStorageFileSet {
- pub(crate) fn new(
- package_map_path: &str,
- flag_map_path: &str,
- flag_val_path: &str,
- read_only: bool,
- ) -> Result<Self> {
- Ok(Self {
- package_map: TestStorageFile::new(package_map_path, read_only)?,
- flag_map: TestStorageFile::new(flag_map_path, read_only)?,
- flag_val: TestStorageFile::new(flag_val_path, read_only)?,
- })
- }
+ Ok(file)
}
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/Android.bp b/tools/aconfig/aconfig_storage_read_api/tests/Android.bp
index ab5ca44..d9cf238 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/Android.bp
+++ b/tools/aconfig/aconfig_storage_read_api/tests/Android.bp
@@ -5,6 +5,7 @@
],
rustlibs: [
"libanyhow",
+ "libaconfig_storage_file",
"libaconfig_storage_read_api",
"libprotobuf",
"libtempfile",
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 b178d83..64239e5 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
@@ -1,12 +1,12 @@
#[cfg(not(feature = "cargo"))]
mod aconfig_storage_rust_test {
+ use aconfig_storage_file::protos::storage_record_pb::write_proto_to_temp_file;
+ use aconfig_storage_file::StorageFileType;
use aconfig_storage_read_api::{
- get_boolean_flag_value_impl, get_flag_offset_impl, get_package_offset_impl,
- get_storage_file_version, PackageOffset, ProtoStorageFiles,
+ get_boolean_flag_value, get_flag_offset, get_package_offset, get_storage_file_version,
+ mapped_file::get_mapped_file, PackageOffset,
};
- use protobuf::Message;
use std::fs;
- use std::io::Write;
use tempfile::NamedTempFile;
pub fn copy_to_ro_temp_file(source_file: &str) -> NamedTempFile {
@@ -19,11 +19,11 @@
file
}
- fn write_storage_location_file(
- package_table: &NamedTempFile,
- flag_table: &NamedTempFile,
- flag_value: &NamedTempFile,
- ) -> NamedTempFile {
+ fn create_test_storage_files() -> [NamedTempFile; 4] {
+ let package_map = copy_to_ro_temp_file("./package.map");
+ let flag_map = copy_to_ro_temp_file("./flag.map");
+ let flag_val = copy_to_ro_temp_file("./flag.val");
+
let text_proto = format!(
r#"
files {{
@@ -35,91 +35,20 @@
timestamp: 12345
}}
"#,
- package_table.path().display(),
- flag_table.path().display(),
- flag_value.path().display(),
+ package_map.path().display(),
+ flag_map.path().display(),
+ flag_val.path().display()
);
-
- 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
+ let pb_file = write_proto_to_temp_file(&text_proto).unwrap();
+ [package_map, flag_map, flag_val, pb_file]
}
#[test]
- fn test_package_offset_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
-
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
-
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_1",
- )
- .unwrap()
- .unwrap();
- let expected_package_offset = PackageOffset { package_id: 0, boolean_offset: 0 };
- assert_eq!(package_offset, expected_package_offset);
-
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_2",
- )
- .unwrap()
- .unwrap();
- let expected_package_offset = PackageOffset { package_id: 1, boolean_offset: 3 };
- assert_eq!(package_offset, expected_package_offset);
-
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_4",
- )
- .unwrap()
- .unwrap();
- let expected_package_offset = PackageOffset { package_id: 2, boolean_offset: 6 };
- assert_eq!(package_offset, expected_package_offset);
-
- let package_offset = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_3",
- )
- .unwrap();
- assert_eq!(package_offset, None);
- }
-
- #[test]
- fn test_invalid_package_offset_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
-
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
-
- let package_offset_option = get_package_offset_impl(
- &file_full_path,
- "system",
- "com.android.aconfig.storage.test_3",
- )
- .unwrap();
- assert_eq!(package_offset_option, None);
-
- let err = get_package_offset_impl(
- &file_full_path,
- "vendor",
- "com.android.aconfig.storage.test_1",
- )
- .unwrap_err();
+ fn test_unavailable_stoarge() {
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let err =
+ get_mapped_file(&pb_file_path, "vendor", StorageFileType::PackageMap).unwrap_err();
assert_eq!(
format!("{:?}", err),
"StorageFileNotFound(Storage file does not exist for vendor)"
@@ -127,13 +56,51 @@
}
#[test]
- fn test_flag_offset_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
+ fn test_package_offset_query() {
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let package_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap();
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_1")
+ .unwrap()
+ .unwrap();
+ let expected_package_offset = PackageOffset { package_id: 0, boolean_offset: 0 };
+ assert_eq!(package_offset, expected_package_offset);
+
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_2")
+ .unwrap()
+ .unwrap();
+ let expected_package_offset = PackageOffset { package_id: 1, boolean_offset: 3 };
+ assert_eq!(package_offset, expected_package_offset);
+
+ let package_offset =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_4")
+ .unwrap()
+ .unwrap();
+ let expected_package_offset = PackageOffset { package_id: 2, boolean_offset: 6 };
+ assert_eq!(package_offset, expected_package_offset);
+ }
+
+ #[test]
+ fn test_none_exist_package_offset_query() {
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let package_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::PackageMap).unwrap();
+ let package_offset_option =
+ get_package_offset(&package_mapped_file, "com.android.aconfig.storage.test_3").unwrap();
+ assert_eq!(package_offset_option, None);
+ }
+
+ #[test]
+ fn test_flag_offset_query() {
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap();
let baseline = vec![
(0, "enabled_ro", 1u16),
@@ -147,7 +114,7 @@
];
for (package_id, flag_name, expected_offset) in baseline.into_iter() {
let flag_offset =
- get_flag_offset_impl(&file_full_path, "system", package_id, flag_name)
+ get_flag_offset(&flag_mapped_file, package_id, flag_name)
.unwrap()
.unwrap();
assert_eq!(flag_offset, expected_offset);
@@ -155,62 +122,44 @@
}
#[test]
- fn test_invalid_flag_offset_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
-
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
+ fn test_none_exist_flag_offset_query() {
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_mapped_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagMap).unwrap();
let flag_offset_option =
- get_flag_offset_impl(&file_full_path, "system", 0, "none_exist").unwrap();
+ get_flag_offset(&flag_mapped_file, 0, "none_exist").unwrap();
assert_eq!(flag_offset_option, None);
let flag_offset_option =
- get_flag_offset_impl(&file_full_path, "system", 3, "enabled_ro").unwrap();
+ get_flag_offset(&flag_mapped_file, 3, "enabled_ro").unwrap();
assert_eq!(flag_offset_option, None);
-
- let err = get_flag_offset_impl(&file_full_path, "vendor", 0, "enabled_ro").unwrap_err();
- assert_eq!(
- format!("{:?}", err),
- "StorageFileNotFound(Storage file does not exist for vendor)"
- );
}
#[test]
fn test_boolean_flag_value_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
-
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_value_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap();
let baseline: Vec<bool> = vec![false; 8];
for (offset, expected_value) in baseline.into_iter().enumerate() {
let flag_value =
- get_boolean_flag_value_impl(&file_full_path, "system", offset as u32).unwrap();
+ get_boolean_flag_value(&flag_value_file, offset as u32).unwrap();
assert_eq!(flag_value, expected_value);
}
}
#[test]
fn test_invalid_boolean_flag_value_query() {
- let package_table = copy_to_ro_temp_file("./package.map");
- let flag_table = copy_to_ro_temp_file("./flag.map");
- let flag_value = copy_to_ro_temp_file("./flag.val");
+ let [_package_map, _flag_map, _flag_val, pb_file] = create_test_storage_files();
+ let pb_file_path = pb_file.path().display().to_string();
+ let flag_value_file =
+ get_mapped_file(&pb_file_path, "system", StorageFileType::FlagVal).unwrap();
- let file = write_storage_location_file(&package_table, &flag_table, &flag_value);
- let file_full_path = file.path().display().to_string();
-
- let err = get_boolean_flag_value_impl(&file_full_path, "vendor", 0u32).unwrap_err();
- assert_eq!(
- format!("{:?}", err),
- "StorageFileNotFound(Storage file does not exist for vendor)"
- );
-
- let err = get_boolean_flag_value_impl(&file_full_path, "system", 8u32).unwrap_err();
+ let err = get_boolean_flag_value(&flag_value_file, 8u32).unwrap_err();
assert_eq!(
format!("{:?}", err),
"InvalidStorageFileOffset(Flag value offset goes beyond the end of the file.)"