Merge "aconfig: return stored flag type enum as a part of flag offset info" into main
diff --git a/tools/aconfig/aconfig_storage_file/src/lib.rs b/tools/aconfig/aconfig_storage_file/src/lib.rs
index 5b8758c..a7b977a 100644
--- a/tools/aconfig/aconfig_storage_file/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_file/src/lib.rs
@@ -103,6 +103,7 @@
 }
 
 /// Flag type enum as stored by storage file
+/// ONLY APPEND, NEVER REMOVE FOR BACKWARD COMPATOBILITY. THE MAX IS U16.
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum StoredFlagType {
     ReadWriteBoolean = 0,
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 2c7c089..ad5656a 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
@@ -155,7 +155,8 @@
   if (offset_cxx.query_success) {
     auto offset = FlagOffset();
     offset.flag_exists = offset_cxx.flag_exists;
-    offset.flag_offset = offset_cxx.flag_offset;
+    offset.flag_type = static_cast<StoredFlagType>(offset_cxx.flag_type);
+    offset.flag_id = offset_cxx.flag_id;
     return offset;
   } else {
    return Error() << offset_cxx.error_message;
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 7a9d6a9..b2a6e41 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
@@ -27,10 +27,18 @@
   uint32_t boolean_offset;
 };
 
+/// Flag type enum, to be consistent with the one defined in aconfig_storage_file/src/lib.rs
+enum StoredFlagType {
+  ReadWriteBoolean = 0,
+  ReadOnlyBoolean = 1,
+  FixedReadOnlyBoolean = 2,
+};
+
 /// Flag offset query result
 struct FlagOffset {
   bool flag_exists;
-  uint16_t flag_offset;
+  StoredFlagType flag_type;
+  uint16_t flag_id;
 };
 
 /// DO NOT USE APIS IN THE FOLLOWING NAMESPACE DIRECTLY
@@ -83,7 +91,7 @@
     MappedStorageFile const& file,
     uint32_t offset);
 
-/// Flag info enum, to be consistent with the one defined in src/lib.rs
+/// Flag info enum, to be consistent with the one defined in aconfig_storage_file/src/lib.rs
 enum FlagInfoBit {
   IsSticky = 1<<0,
   IsReadWrite = 1<<1,
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 a251b41..ad8e394 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
@@ -18,11 +18,16 @@
 
 use crate::{AconfigStorageError, FILE_VERSION};
 use aconfig_storage_file::{
-    flag_table::FlagTableHeader, flag_table::FlagTableNode, read_u32_from_bytes,
+    flag_table::FlagTableHeader, flag_table::FlagTableNode, read_u32_from_bytes, StoredFlagType,
 };
 use anyhow::anyhow;
 
-pub type FlagOffset = u16;
+/// Flag table query return
+#[derive(PartialEq, Debug)]
+pub struct FlagOffset {
+    pub flag_type: StoredFlagType,
+    pub flag_id: u16,
+}
 
 /// Query flag within package offset
 pub fn find_flag_offset(
@@ -53,7 +58,10 @@
     loop {
         let interpreted_node = FlagTableNode::from_bytes(&buf[flag_node_offset..])?;
         if interpreted_node.package_id == package_id && interpreted_node.flag_name == flag {
-            return Ok(Some(interpreted_node.flag_id));
+            return Ok(Some(FlagOffset {
+                flag_type: interpreted_node.flag_type,
+                flag_id: interpreted_node.flag_id,
+            }));
         }
         match interpreted_node.next_offset {
             Some(offset) => flag_node_offset = offset as usize,
@@ -72,19 +80,20 @@
     fn test_flag_query() {
         let flag_table = create_test_flag_table().into_bytes();
         let baseline = vec![
-            (0, "enabled_ro", 1u16),
-            (0, "enabled_rw", 2u16),
-            (1, "disabled_ro", 0u16),
-            (2, "enabled_ro", 1u16),
-            (1, "enabled_fixed_ro", 1u16),
-            (1, "enabled_ro", 2u16),
-            (2, "enabled_fixed_ro", 0u16),
-            (0, "disabled_rw", 0u16),
+            (0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
+            (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
+            (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
+            (1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
+            (2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
+            (0, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
         ];
-        for (package_id, flag_name, expected_offset) in baseline.into_iter() {
+        for (package_id, flag_name, flag_type, flag_id) in baseline.into_iter() {
             let flag_offset =
                 find_flag_offset(&flag_table[..], package_id, flag_name).unwrap().unwrap();
-            assert_eq!(flag_offset, expected_offset);
+            assert_eq!(flag_offset.flag_type, flag_type);
+            assert_eq!(flag_offset.flag_id, flag_id);
         }
     }
 
diff --git a/tools/aconfig/aconfig_storage_read_api/src/lib.rs b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
index 280c332..960446d 100644
--- a/tools/aconfig/aconfig_storage_read_api/src/lib.rs
+++ b/tools/aconfig/aconfig_storage_read_api/src/lib.rs
@@ -187,7 +187,8 @@
         pub query_success: bool,
         pub error_message: String,
         pub flag_exists: bool,
-        pub flag_offset: u16,
+        pub flag_type: u16,
+        pub flag_id: u16,
     }
 
     // Flag value query return for cc interlop
@@ -214,7 +215,10 @@
 
         pub fn get_boolean_flag_value_cxx(file: &[u8], offset: u32) -> BooleanFlagValueQueryCXX;
 
-        pub fn get_boolean_flag_attribute_cxx(file: &[u8], offset: u32) -> BooleanFlagAttributeQueryCXX;
+        pub fn get_boolean_flag_attribute_cxx(
+            file: &[u8],
+            offset: u32,
+        ) -> BooleanFlagAttributeQueryCXX;
     }
 }
 
@@ -258,20 +262,23 @@
                     query_success: true,
                     error_message: String::from(""),
                     flag_exists: true,
-                    flag_offset: offset,
+                    flag_type: offset.flag_type as u16,
+                    flag_id: offset.flag_id,
                 },
                 None => Self {
                     query_success: true,
                     error_message: String::from(""),
                     flag_exists: false,
-                    flag_offset: 0,
+                    flag_type: 0u16,
+                    flag_id: 0u16,
                 },
             },
             Err(errmsg) => Self {
                 query_success: false,
                 error_message: format!("{:?}", errmsg),
                 flag_exists: false,
-                flag_offset: 0,
+                flag_type: 0u16,
+                flag_id: 0u16,
             },
         }
     }
@@ -344,7 +351,10 @@
 }
 
 /// Get boolean flag attribute cc interlop
-pub fn get_boolean_flag_attribute_cxx(file: &[u8], offset: u32) -> ffi::BooleanFlagAttributeQueryCXX {
+pub fn get_boolean_flag_attribute_cxx(
+    file: &[u8],
+    offset: u32,
+) -> ffi::BooleanFlagAttributeQueryCXX {
     ffi::BooleanFlagAttributeQueryCXX::new(find_boolean_flag_attribute(file, offset))
 }
 
@@ -359,7 +369,7 @@
     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 aconfig_storage_file::FlagInfoBit;
+    use aconfig_storage_file::{FlagInfoBit, StoredFlagType};
     use tempfile::NamedTempFile;
 
     fn create_test_storage_files() -> [NamedTempFile; 5] {
@@ -429,19 +439,20 @@
             unsafe { get_mapped_file(&pb_file_path, "mockup", StorageFileType::FlagMap).unwrap() };
 
         let baseline = vec![
-            (0, "enabled_ro", 1u16),
-            (0, "enabled_rw", 2u16),
-            (1, "disabled_ro", 0u16),
-            (2, "enabled_ro", 1u16),
-            (1, "enabled_fixed_ro", 1u16),
-            (1, "enabled_ro", 2u16),
-            (2, "enabled_fixed_ro", 0u16),
-            (0, "disabled_rw", 0u16),
+            (0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
+            (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
+            (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
+            (1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
+            (2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
+            (0, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
         ];
-        for (package_id, flag_name, expected_offset) in baseline.into_iter() {
+        for (package_id, flag_name, flag_type, flag_id) in baseline.into_iter() {
             let flag_offset =
                 get_flag_offset(&flag_mapped_file, package_id, flag_name).unwrap().unwrap();
-            assert_eq!(flag_offset, expected_offset);
+            assert_eq!(flag_offset.flag_type, flag_type);
+            assert_eq!(flag_offset.flag_id, flag_id);
         }
     }
 
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 6bc3723..3fd0173 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
@@ -161,21 +161,22 @@
       storage_record_pb, "mockup", api::StorageFileType::flag_map);
   ASSERT_TRUE(mapped_file.ok());
 
-  auto baseline = std::vector<std::tuple<int, std::string, int>>{
-    {0, "enabled_ro", 1},
-    {0, "enabled_rw", 2},
-    {1, "disabled_ro", 0},
-    {2, "enabled_ro", 1},
-    {1, "enabled_fixed_ro", 1},
-    {1, "enabled_ro", 2},
-    {2, "enabled_fixed_ro", 0},
-    {0, "disabled_rw", 0},
+  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},
+    {1, "enabled_fixed_ro", api::StoredFlagType::FixedReadOnlyBoolean, 1},
+    {1, "enabled_ro", api::StoredFlagType::ReadOnlyBoolean, 2},
+    {2, "enabled_fixed_ro", api::StoredFlagType::FixedReadOnlyBoolean, 0},
+    {0, "disabled_rw", api::StoredFlagType::ReadWriteBoolean, 0},
   };
-  for (auto const&[package_id, flag_name, expected_offset] : baseline) {
+  for (auto const&[package_id, flag_name, flag_type, flag_id] : baseline) {
     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);
+    ASSERT_EQ(offset->flag_type, flag_type);
+    ASSERT_EQ(offset->flag_id, flag_id);
   }
 }
 
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 5b35a84..21caead 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,7 +1,7 @@
 #[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::{FlagInfoBit, StorageFileType};
+    use aconfig_storage_file::{FlagInfoBit, StorageFileType, StoredFlagType};
     use aconfig_storage_read_api::{
         get_boolean_flag_attribute, get_boolean_flag_value, get_flag_offset, get_package_offset,
         get_storage_file_version, mapped_file::get_mapped_file, PackageOffset,
@@ -114,19 +114,20 @@
             unsafe { get_mapped_file(&pb_file_path, "mockup", StorageFileType::FlagMap).unwrap() };
 
         let baseline = vec![
-            (0, "enabled_ro", 1u16),
-            (0, "enabled_rw", 2u16),
-            (1, "disabled_ro", 0u16),
-            (2, "enabled_ro", 1u16),
-            (1, "enabled_fixed_ro", 1u16),
-            (1, "enabled_ro", 2u16),
-            (2, "enabled_fixed_ro", 0u16),
-            (0, "disabled_rw", 0u16),
+            (0, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (0, "enabled_rw", StoredFlagType::ReadWriteBoolean, 2u16),
+            (1, "disabled_ro", StoredFlagType::ReadOnlyBoolean, 0u16),
+            (2, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 1u16),
+            (1, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 1u16),
+            (1, "enabled_ro", StoredFlagType::ReadOnlyBoolean, 2u16),
+            (2, "enabled_fixed_ro", StoredFlagType::FixedReadOnlyBoolean, 0u16),
+            (0, "disabled_rw", StoredFlagType::ReadWriteBoolean, 0u16),
         ];
-        for (package_id, flag_name, expected_offset) in baseline.into_iter() {
+        for (package_id, flag_name, flag_type, flag_id) in baseline.into_iter() {
             let flag_offset =
                 get_flag_offset(&flag_mapped_file, package_id, flag_name).unwrap().unwrap();
-            assert_eq!(flag_offset, expected_offset);
+            assert_eq!(flag_offset.flag_type, flag_type);
+            assert_eq!(flag_offset.flag_id, flag_id);
         }
     }