update_engine: Multi-level |PrefsInterface::CreateSubKey()|
Currently, |PrefsInterface::CreateSubKey()| is limited to always provide
a namespace and subpref, but this can be generalized to a multi-level
namespace alongside a supplied key.
BUG=chromium:928805
TEST=FEATURES=test emerge-$B update_engine
Change-Id: Ib81e93e8319714caa85cd2fe6495d3cb9b0e82ed
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2195623
Tested-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Andrew Lassalle <andrewlassalle@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
Commit-Queue: Jae Hoon Kim <kimjae@chromium.org>
diff --git a/common/prefs.cc b/common/prefs.cc
index 6a33037..6db01b7 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -28,11 +28,27 @@
#include "update_engine/common/utils.h"
using std::string;
+using std::vector;
namespace chromeos_update_engine {
const char kKeySeparator = '/';
+namespace {
+
+void DeleteEmptyDirectories(const base::FilePath& path) {
+ base::FileEnumerator path_enum(
+ path, false /* recursive */, base::FileEnumerator::DIRECTORIES);
+ for (base::FilePath dir_path = path_enum.Next(); !dir_path.empty();
+ dir_path = path_enum.Next()) {
+ DeleteEmptyDirectories(dir_path);
+ if (base::IsDirectoryEmpty(dir_path))
+ base::DeleteFile(dir_path, false);
+ }
+}
+
+} // namespace
+
bool PrefsBase::GetString(const string& key, string* value) const {
return storage_->GetKey(key, value);
}
@@ -108,11 +124,8 @@
observers_for_key.erase(observer_it);
}
-string PrefsInterface::CreateSubKey(const string& name_space,
- const string& sub_pref,
- const string& key) {
- return base::JoinString({name_space, sub_pref, key},
- string(1, kKeySeparator));
+string PrefsInterface::CreateSubKey(const vector<string>& ns_and_key) {
+ return base::JoinString(ns_and_key, string(1, kKeySeparator));
}
// Prefs
@@ -124,23 +137,7 @@
bool Prefs::FileStorage::Init(const base::FilePath& prefs_dir) {
prefs_dir_ = prefs_dir;
// Delete empty directories. Ignore errors when deleting empty directories.
- base::FileEnumerator namespace_enum(
- prefs_dir_, false /* recursive */, base::FileEnumerator::DIRECTORIES);
- for (base::FilePath namespace_path = namespace_enum.Next();
- !namespace_path.empty();
- namespace_path = namespace_enum.Next()) {
- base::FileEnumerator sub_pref_enum(namespace_path,
- false /* recursive */,
- base::FileEnumerator::DIRECTORIES);
- for (base::FilePath sub_pref_path = sub_pref_enum.Next();
- !sub_pref_path.empty();
- sub_pref_path = sub_pref_enum.Next()) {
- if (base::IsDirectoryEmpty(sub_pref_path))
- base::DeleteFile(sub_pref_path, false);
- }
- if (base::IsDirectoryEmpty(namespace_path))
- base::DeleteFile(namespace_path, false);
- }
+ DeleteEmptyDirectories(prefs_dir_);
return true;
}
diff --git a/common/prefs_interface.h b/common/prefs_interface.h
index b559697..3aad480 100644
--- a/common/prefs_interface.h
+++ b/common/prefs_interface.h
@@ -20,6 +20,7 @@
#include <stdint.h>
#include <string>
+#include <vector>
namespace chromeos_update_engine {
@@ -80,9 +81,7 @@
virtual bool Delete(const std::string& key) = 0;
// Creates a key which is part of a sub preference.
- static std::string CreateSubKey(const std::string& name_space,
- const std::string& sub_pref,
- const std::string& key);
+ static std::string CreateSubKey(const std::vector<std::string>& ns_with_key);
// Add an observer to watch whenever the given |key| is modified. The
// OnPrefSet() and OnPrefDelete() methods will be called whenever any of the
diff --git a/common/prefs_unittest.cc b/common/prefs_unittest.cc
index f226949..24a62b5 100644
--- a/common/prefs_unittest.cc
+++ b/common/prefs_unittest.cc
@@ -62,17 +62,27 @@
TEST(Prefs, Init) {
Prefs prefs;
- const string name_space = "ns";
+ const string ns1 = "ns1";
+ const string ns2A = "ns2A";
+ const string ns2B = "ns2B";
const string sub_pref = "sp";
base::ScopedTempDir temp_dir;
ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- base::FilePath namespace_path = temp_dir.GetPath().Append(name_space);
+ auto ns1_path = temp_dir.GetPath().Append(ns1);
+ auto ns2A_path = ns1_path.Append(ns2A);
+ auto ns2B_path = ns1_path.Append(ns2B);
+ auto sub_pref_path = ns2A_path.Append(sub_pref);
- EXPECT_TRUE(base::CreateDirectory(namespace_path.Append(sub_pref)));
- EXPECT_TRUE(base::PathExists(namespace_path.Append(sub_pref)));
+ EXPECT_TRUE(base::CreateDirectory(ns2B_path));
+ EXPECT_TRUE(base::PathExists(ns2B_path));
+
+ EXPECT_TRUE(base::CreateDirectory(sub_pref_path));
+ EXPECT_TRUE(base::PathExists(sub_pref_path));
+
+ EXPECT_TRUE(base::PathExists(ns1_path));
ASSERT_TRUE(prefs.Init(temp_dir.GetPath()));
- EXPECT_FALSE(base::PathExists(namespace_path));
+ EXPECT_FALSE(base::PathExists(ns1_path));
}
TEST_F(PrefsTest, GetFileNameForKey) {
@@ -99,9 +109,9 @@
const string sub_pref2 = "sp2";
const string sub_key = "sk";
- EXPECT_EQ(PrefsInterface::CreateSubKey(name_space, sub_pref1, sub_key),
+ EXPECT_EQ(PrefsInterface::CreateSubKey({name_space, sub_pref1, sub_key}),
"ns/sp1/sk");
- EXPECT_EQ(PrefsInterface::CreateSubKey(name_space, sub_pref2, sub_key),
+ EXPECT_EQ(PrefsInterface::CreateSubKey({name_space, sub_pref2, sub_key}),
"ns/sp2/sk");
}
@@ -312,8 +322,8 @@
const string sub_pref = "sp";
const string sub_key1 = "sk1";
const string sub_key2 = "sk2";
- auto key1 = prefs_.CreateSubKey(name_space, sub_pref, sub_key1);
- auto key2 = prefs_.CreateSubKey(name_space, sub_pref, sub_key2);
+ auto key1 = prefs_.CreateSubKey({name_space, sub_pref, sub_key1});
+ auto key2 = prefs_.CreateSubKey({name_space, sub_pref, sub_key2});
base::FilePath sub_pref_path = prefs_dir_.Append(name_space).Append(sub_pref);
ASSERT_TRUE(prefs_.SetInt64(key1, 0));
@@ -350,7 +360,7 @@
prefs_.Delete(kKey);
testing::Mock::VerifyAndClearExpectations(&mock_obserser);
- auto key1 = prefs_.CreateSubKey("ns", "sp1", "key1");
+ auto key1 = prefs_.CreateSubKey({"ns", "sp1", "key1"});
prefs_.AddObserver(key1, &mock_obserser);
EXPECT_CALL(mock_obserser, OnPrefSet(key1));
@@ -424,7 +434,7 @@
EXPECT_FALSE(prefs_.Exists(kKey));
EXPECT_TRUE(prefs_.Delete(kKey));
- auto key = prefs_.CreateSubKey("ns", "sp", "sk");
+ auto key = prefs_.CreateSubKey({"ns", "sp", "sk"});
ASSERT_TRUE(prefs_.SetInt64(key, 0));
EXPECT_TRUE(prefs_.Exists(key));
EXPECT_TRUE(prefs_.Delete(kKey));
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index c9b8aa0..8728f72 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -421,13 +421,13 @@
PrefsInterface* prefs = system_state_->prefs();
// Reset the active metadata value to |kPingInactiveValue|.
auto active_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
if (!prefs->SetInt64(active_key, kPingInactiveValue))
LOG(ERROR) << "Failed to set the value of ping metadata '" << active_key
<< "'.";
auto last_rollcall_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
if (!prefs->SetString(last_rollcall_key, parser_data.daystart_elapsed_days))
LOG(ERROR) << "Failed to set the value of ping metadata '"
<< last_rollcall_key << "'.";
@@ -436,7 +436,7 @@
// Write the value of elapsed_days into |kPrefsPingLastActive| only if
// the previous ping was an active one.
auto last_active_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
if (!prefs->SetString(last_active_key, parser_data.daystart_elapsed_days))
LOG(ERROR) << "Failed to set the value of ping metadata '"
<< last_active_key << "'.";
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index e1f5ef9..765af4f 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -453,11 +453,11 @@
OmahaRequestActionTest::SetUp();
dlc_id_ = "dlc0";
active_key_ = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id_, kPrefsPingActive);
+ {kDlcPrefsSubDir, dlc_id_, kPrefsPingActive});
last_active_key_ = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id_, kPrefsPingLastActive);
+ {kDlcPrefsSubDir, dlc_id_, kPrefsPingLastActive});
last_rollcall_key_ = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id_, kPrefsPingLastRollcall);
+ {kDlcPrefsSubDir, dlc_id_, kPrefsPingLastRollcall});
tuc_params_.http_response =
"<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
diff --git a/update_attempter.cc b/update_attempter.cc
index 6324a48..7479134 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -663,7 +663,7 @@
PrefsInterface* prefs = system_state_->prefs();
for (auto& sub_key :
{kPrefsPingActive, kPrefsPingLastActive, kPrefsPingLastRollcall}) {
- auto key = prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, sub_key);
+ auto key = prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, sub_key});
if (!prefs->Delete(key))
failures.emplace_back(sub_key);
}
@@ -684,7 +684,7 @@
PrefsInterface* prefs = system_state_->prefs();
if (is_active) {
auto ping_active_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
if (!prefs->SetInt64(ping_active_key, kPingActiveValue)) {
LOG(ERROR) << "Failed to set the value of ping metadata '"
<< kPrefsPingActive << "'.";
@@ -740,17 +740,17 @@
// install or might not really be active yet.
dlc_params.ping_active = kPingActiveValue;
auto ping_active_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
if (!prefs->GetInt64(ping_active_key, &dlc_params.ping_active) ||
dlc_params.ping_active != kPingActiveValue) {
dlc_params.ping_active = kPingInactiveValue;
}
auto ping_last_active_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ prefs->CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
dlc_params.ping_date_last_active = GetPingMetadata(ping_last_active_key);
- auto ping_last_rollcall_key =
- prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ auto ping_last_rollcall_key = prefs->CreateSubKey(
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
dlc_params.ping_date_last_rollcall =
GetPingMetadata(ping_last_rollcall_key);
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 3a1646f..745bcc2 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -2390,10 +2390,10 @@
// When the DLC gets installed, a ping is not sent, therefore we don't store
// the values sent by Omaha.
auto last_active_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_active_key));
auto last_rollcall_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_rollcall_key));
}
@@ -2430,11 +2430,11 @@
// Write non numeric values in the metadata files.
auto active_key =
- PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
auto last_active_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
auto last_rollcall_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
fake_system_state_.prefs()->SetString(active_key, "z2yz");
fake_system_state_.prefs()->SetString(last_active_key, "z2yz");
fake_system_state_.prefs()->SetString(last_rollcall_key, "z2yz");
@@ -2463,11 +2463,11 @@
// Write numeric values in the metadata files.
auto active_key =
- PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
auto last_active_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
auto last_rollcall_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
fake_system_state_.prefs()->SetInt64(active_key, 1);
fake_system_state_.prefs()->SetInt64(last_active_key, 78);
@@ -2492,11 +2492,11 @@
FakePrefs fake_prefs;
fake_system_state_.set_prefs(&fake_prefs);
auto active_key =
- PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
auto last_active_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive});
auto last_rollcall_key = PrefsInterface::CreateSubKey(
- kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ {kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall});
fake_system_state_.prefs()->SetInt64(active_key, kPingInactiveValue);
fake_system_state_.prefs()->SetInt64(last_active_key, 0);
fake_system_state_.prefs()->SetInt64(last_rollcall_key, 0);
@@ -2524,7 +2524,7 @@
attempter_.SetDlcActiveValue(true, dlc_id);
int64_t temp_int;
auto active_key =
- PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, kPrefsPingActive});
EXPECT_TRUE(fake_system_state_.prefs()->Exists(active_key));
EXPECT_TRUE(fake_system_state_.prefs()->GetInt64(active_key, &temp_int));
EXPECT_EQ(temp_int, kPingActiveValue);
@@ -2537,13 +2537,13 @@
auto sub_keys = {
kPrefsPingActive, kPrefsPingLastActive, kPrefsPingLastRollcall};
for (auto& sub_key : sub_keys) {
- auto key = PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, sub_key);
+ auto key = PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, sub_key});
fake_system_state_.prefs()->SetInt64(key, 1);
EXPECT_TRUE(fake_system_state_.prefs()->Exists(key));
}
attempter_.SetDlcActiveValue(false, dlc_id);
for (auto& sub_key : sub_keys) {
- auto key = PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, sub_key);
+ auto key = PrefsInterface::CreateSubKey({kDlcPrefsSubDir, dlc_id, sub_key});
EXPECT_FALSE(fake_system_state_.prefs()->Exists(key));
}
}