Merge "Move staging value application logic to persistent_properties and add unit tests" into main

init/persistent_properties.cpp

@@ -46,6 +46,13 @@ namespace {
 
 constexpr const char kLegacyPersistentPropertyDir[] = "/data/property";
 
+void AddPersistentProperty(const std::string& name, const std::string& value,
+                           PersistentProperties* persistent_properties) {
+    auto persistent_property_record = persistent_properties->add_properties();
+    persistent_property_record->set_name(name);
+    persistent_property_record->set_value(value);
+}
+
 Result<PersistentProperties> LoadLegacyPersistentProperties() {
     std::unique_ptr<DIR, decltype(&closedir)> dir(opendir(kLegacyPersistentPropertyDir), closedir);
     if (!dir) {
@@ -146,13 +153,6 @@ Result<PersistentProperties> ParsePersistentPropertyFile(const std::string& file
 
 }  // namespace
 
-void AddPersistentProperty(const std::string& name, const std::string& value,
-                           PersistentProperties* persistent_properties) {
-    auto persistent_property_record = persistent_properties->add_properties();
-    persistent_property_record->set_name(name);
-    persistent_property_record->set_value(value);
-}
-
 Result<PersistentProperties> LoadPersistentPropertyFile() {
     auto file_contents = ReadPersistentPropertyFile();
     if (!file_contents.ok()) return file_contents.error();
@@ -266,8 +266,57 @@ PersistentProperties LoadPersistentProperties() {
         }
     }
 
-    return *persistent_properties;
+    // loop over to find all staged props
+    auto const staged_prefix = std::string_view("next_boot.");
+    auto staged_props = std::unordered_map<std::string, std::string>();
+    for (const auto& property_record : persistent_properties->properties()) {
+        auto const& prop_name = property_record.name();
+        auto const& prop_value = property_record.value();
+        if (StartsWith(prop_name, staged_prefix)) {
+            auto actual_prop_name = prop_name.substr(staged_prefix.size());
+            staged_props[actual_prop_name] = prop_value;
+        }
+    }
+
+    if (staged_props.empty()) {
+        return *persistent_properties;
+    }
+
+    // if has staging, apply staging and perserve the original prop order
+    PersistentProperties updated_persistent_properties;
+    for (const auto& property_record : persistent_properties->properties()) {
+        auto const& prop_name = property_record.name();
+        auto const& prop_value = property_record.value();
+
+        // don't include staged props anymore
+        if (StartsWith(prop_name, staged_prefix)) {
+            continue;
+        }
+
+        auto iter = staged_props.find(prop_name);
+        if (iter != staged_props.end()) {
+            AddPersistentProperty(prop_name, iter->second, &updated_persistent_properties);
+            staged_props.erase(iter);
+        } else {
+            AddPersistentProperty(prop_name, prop_value, &updated_persistent_properties);
+        }
+    }
+
+    // add any additional staged props
+    for (auto const& [prop_name, prop_value] : staged_props) {
+        AddPersistentProperty(prop_name, prop_value, &updated_persistent_properties);
+    }
+
+    // write current updated persist prop file
+    auto result = WritePersistentPropertyFile(updated_persistent_properties);
+    if (!result.ok()) {
+        LOG(ERROR) << "Could not store persistent property: " << result.error();
+    }
+
+    return updated_persistent_properties;
 }
 
+
+
 }  // namespace init
 }  // namespace android

init/persistent_properties.h

@@ -25,8 +25,6 @@
 namespace android {
 namespace init {
 
-void AddPersistentProperty(const std::string& name, const std::string& value,
-                           PersistentProperties* persistent_properties);
 PersistentProperties LoadPersistentProperties();
 void WritePersistentProperty(const std::string& name, const std::string& value);
 PersistentProperties LoadPersistentPropertiesFromMemory();

init/persistent_properties_test.cpp

@@ -178,5 +178,37 @@ TEST(persistent_properties, RejectNonPersistProperty) {
     EXPECT_FALSE(it == read_back_properties.properties().end());
 }
 
+TEST(persistent_properties, StagedPersistProperty) {
+    TemporaryFile tf;
+    ASSERT_TRUE(tf.fd != -1);
+    persistent_property_filename = tf.path;
+
+    std::vector<std::pair<std::string, std::string>> persistent_properties = {
+        {"persist.sys.locale", "en-US"},
+        {"next_boot.persist.test.numbers", "54321"},
+        {"persist.sys.timezone", "America/Los_Angeles"},
+        {"persist.test.numbers", "12345"},
+        {"next_boot.persist.test.extra", "abc"},
+    };
+
+    ASSERT_RESULT_OK(
+            WritePersistentPropertyFile(VectorToPersistentProperties(persistent_properties)));
+
+    std::vector<std::pair<std::string, std::string>> expected_persistent_properties = {
+        {"persist.sys.locale", "en-US"},
+        {"persist.sys.timezone", "America/Los_Angeles"},
+        {"persist.test.numbers", "54321"},
+        {"persist.test.extra", "abc"},
+    };
+
+    // lock down that staged props are applied
+    auto first_read_back_properties = LoadPersistentProperties();
+    CheckPropertiesEqual(expected_persistent_properties, first_read_back_properties);
+
+    // lock down that other props are not overwritten
+    auto second_read_back_properties = LoadPersistentProperties();
+    CheckPropertiesEqual(expected_persistent_properties, second_read_back_properties);
+}
+
 }  // namespace init
 }  // namespace android

init/property_service.cpp

@@ -1406,33 +1406,12 @@ static void HandleInitSocket() {
     switch (init_message.msg_case()) {
         case InitMessage::kLoadPersistentProperties: {
             load_override_properties();
-            // Read persistent properties after all default values have been loaded.
-            // Apply staged and persistent properties
-            bool has_staged_prop = false;
-            auto const staged_prefix = std::string_view("next_boot.");
 
             auto persistent_properties = LoadPersistentProperties();
             for (const auto& property_record : persistent_properties.properties()) {
                 auto const& prop_name = property_record.name();
                 auto const& prop_value = property_record.value();
-
+                InitPropertySet(prop_name, prop_value);
-                if (StartsWith(prop_name, staged_prefix)) {
-                  has_staged_prop = true;
-                  auto actual_prop_name = prop_name.substr(staged_prefix.size());
-                  InitPropertySet(actual_prop_name, prop_value);
-                } else {
-                  InitPropertySet(prop_name, prop_value);
-                }
-            }
-
-            // Update persist prop file if there are staged props
-            if (has_staged_prop) {
-                PersistentProperties props = LoadPersistentPropertiesFromMemory();
-                // write current updated persist prop file
-                auto result = WritePersistentPropertyFile(props);
-                if (!result.ok()) {
-                    LOG(ERROR) << "Could not store persistent property: " << result.error();
-                }
             }
 
             // Apply debug ramdisk special settings after persistent properties are loaded.