Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 79283ef3 authored by Dennis Shen's avatar Dennis Shen
Browse files

Move staging value application logic to persistent_properties and add 

unit tests

1, Previous implementation has the staged prop application done in
property_service, which caused a number of unnecessary changes which
including exposing apis like AddPersistentProperty. In addition, it made
the property_service logic complicated. A better design is to have the
staged value application done while reading the persistent properties
from file. This way, no change to property service. In addition, unit
test is much cleaner and efficient.

2, add a unit test to lock down the behavior. Specifically, it locks down that when a prop is staged, it should be applied the next time when the persistent prop is loaded. In addition, it should lock down that other persistent props are not overwritten.

Bug: b/307752841, b/300111812, b/306062513

Change-Id: I43c603efbb803195065dda3f0bc2145716302bbc
parent 3609bf70

init/persistent_properties.cpp

+57 −8
Original line number Diff line number Diff line
@@ -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() { 
        }

    }



    // 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

+0 −2
Original line number Diff line number Diff line
@@ -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

+32 −0
Original line number Diff line number Diff line
@@ -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

+1 −22
Original line number Diff line number Diff line
@@ -1396,34 +1396,13 @@ 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();



                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.

            if (android::base::GetBoolProperty("ro.force.debuggable", false)) {