update_engine: Change DLC metadata path
Change the location of the DLC metadata from /var/lib/dlc to
/var/lib/update_engine/dlc_prefs/ to make update_engine the owner of
metadata.
BUG=chromium:912666
TEST=cros_workon_make update_engine --test
TEST=install and uninstall DLCs on DUT. Check new prefs path.
Change-Id: I75f5506eee1abc834ad89a7cf363f42e384b695b
Reviewed-on: https://chromium-review.googlesource.com/c/aosp/platform/system/update_engine/+/2140007
Tested-by: Andrew Lassalle <andrewlassalle@chromium.org>
Commit-Queue: Amin Hassani <ahassani@chromium.org>
Reviewed-by: Jae Hoon Kim <kimjae@chromium.org>
Reviewed-by: Amin Hassani <ahassani@chromium.org>
diff --git a/common/constants.cc b/common/constants.cc
index 793ce97..25aa9a8 100644
--- a/common/constants.cc
+++ b/common/constants.cc
@@ -18,8 +18,7 @@
namespace chromeos_update_engine {
-// TODO(andrewlassalle): Move this to the prefs directory.
-const char kDlcMetadataRootpath[] = "/var/lib/dlc/";
+const char kDlcPrefsSubDir[] = "dlc";
const char kPowerwashSafePrefsSubDirectory[] = "update_engine/prefs";
diff --git a/common/constants.h b/common/constants.h
index 44b20b0..67519bd 100644
--- a/common/constants.h
+++ b/common/constants.h
@@ -19,9 +19,8 @@
namespace chromeos_update_engine {
-// The root path of all DLC modules metadata.
-// Keep this in sync with the one in dlcservice.
-extern const char kDlcMetadataRootpath[];
+// The root path of all DLC metadata.
+extern const char kDlcPrefsSubDir[];
// Directory for AU prefs that are preserved across powerwash.
extern const char kPowerwashSafePrefsSubDirectory[];
diff --git a/common/prefs.cc b/common/prefs.cc
index 6d86a50..6a33037 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -18,9 +18,11 @@
#include <algorithm>
+#include <base/files/file_enumerator.h>
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/strings/string_number_conversions.h>
+#include <base/strings/string_split.h>
#include <base/strings/string_util.h>
#include "update_engine/common/utils.h"
@@ -29,6 +31,8 @@
namespace chromeos_update_engine {
+const char kKeySeparator = '/';
+
bool PrefsBase::GetString(const string& key, string* value) const {
return storage_->GetKey(key, value);
}
@@ -104,6 +108,13 @@
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));
+}
+
// Prefs
bool Prefs::Init(const base::FilePath& prefs_dir) {
@@ -112,6 +123,24 @@
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);
+ }
return true;
}
@@ -146,7 +175,7 @@
bool Prefs::FileStorage::DeleteKey(const string& key) {
base::FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- TEST_AND_RETURN_FALSE(base::DeleteFile(filename, false));
+ TEST_AND_RETURN_FALSE(base::DeleteFile(filename, true));
return true;
}
@@ -157,7 +186,7 @@
for (size_t i = 0; i < key.size(); ++i) {
char c = key.at(i);
TEST_AND_RETURN_FALSE(base::IsAsciiAlpha(c) || base::IsAsciiDigit(c) ||
- c == '_' || c == '-');
+ c == '_' || c == '-' || c == kKeySeparator);
}
*filename = prefs_dir_.Append(key);
return true;
diff --git a/common/prefs_interface.h b/common/prefs_interface.h
index 03ae3ec..b559697 100644
--- a/common/prefs_interface.h
+++ b/common/prefs_interface.h
@@ -79,6 +79,11 @@
// this key. Calling with non-existent keys does nothing.
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);
+
// Add an observer to watch whenever the given |key| is modified. The
// OnPrefSet() and OnPrefDelete() methods will be called whenever any of the
// Set*() methods or the Delete() method are called on the given key,
diff --git a/common/prefs_unittest.cc b/common/prefs_unittest.cc
index 3f29319..f226949 100644
--- a/common/prefs_unittest.cc
+++ b/common/prefs_unittest.cc
@@ -31,6 +31,7 @@
using std::string;
using testing::_;
+using testing::ElementsAre;
using testing::Eq;
namespace {
@@ -59,6 +60,21 @@
Prefs prefs_;
};
+TEST(Prefs, Init) {
+ Prefs prefs;
+ const string name_space = "ns";
+ const string sub_pref = "sp";
+
+ base::ScopedTempDir temp_dir;
+ ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
+ base::FilePath namespace_path = temp_dir.GetPath().Append(name_space);
+
+ EXPECT_TRUE(base::CreateDirectory(namespace_path.Append(sub_pref)));
+ EXPECT_TRUE(base::PathExists(namespace_path.Append(sub_pref)));
+ ASSERT_TRUE(prefs.Init(temp_dir.GetPath()));
+ EXPECT_FALSE(base::PathExists(namespace_path));
+}
+
TEST_F(PrefsTest, GetFileNameForKey) {
const char kAllvalidCharsKey[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-";
@@ -77,6 +93,18 @@
EXPECT_FALSE(prefs_.file_storage_.GetFileNameForKey("", &path));
}
+TEST_F(PrefsTest, CreateSubKey) {
+ const string name_space = "ns";
+ const string sub_pref1 = "sp1";
+ const string sub_pref2 = "sp2";
+ const string sub_key = "sk";
+
+ EXPECT_EQ(PrefsInterface::CreateSubKey(name_space, sub_pref1, sub_key),
+ "ns/sp1/sk");
+ EXPECT_EQ(PrefsInterface::CreateSubKey(name_space, sub_pref2, sub_key),
+ "ns/sp2/sk");
+}
+
TEST_F(PrefsTest, GetString) {
const string test_data = "test data";
ASSERT_TRUE(SetValue(kKey, test_data));
@@ -279,6 +307,29 @@
EXPECT_FALSE(prefs_.Exists(kKey));
}
+TEST_F(PrefsTest, SetDeleteSubKey) {
+ const string name_space = "ns";
+ 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);
+ base::FilePath sub_pref_path = prefs_dir_.Append(name_space).Append(sub_pref);
+
+ ASSERT_TRUE(prefs_.SetInt64(key1, 0));
+ ASSERT_TRUE(prefs_.SetInt64(key2, 0));
+ EXPECT_TRUE(base::PathExists(sub_pref_path.Append(sub_key1)));
+ EXPECT_TRUE(base::PathExists(sub_pref_path.Append(sub_key2)));
+
+ ASSERT_TRUE(prefs_.Delete(key1));
+ EXPECT_FALSE(base::PathExists(sub_pref_path.Append(sub_key1)));
+ EXPECT_TRUE(base::PathExists(sub_pref_path.Append(sub_key2)));
+ ASSERT_TRUE(prefs_.Delete(key2));
+ EXPECT_FALSE(base::PathExists(sub_pref_path.Append(sub_key2)));
+ prefs_.Init(prefs_dir_);
+ EXPECT_FALSE(base::PathExists(prefs_dir_.Append(name_space)));
+}
+
class MockPrefsObserver : public PrefsInterface::ObserverInterface {
public:
MOCK_METHOD1(OnPrefSet, void(const string&));
@@ -299,6 +350,19 @@
prefs_.Delete(kKey);
testing::Mock::VerifyAndClearExpectations(&mock_obserser);
+ auto key1 = prefs_.CreateSubKey("ns", "sp1", "key1");
+ prefs_.AddObserver(key1, &mock_obserser);
+
+ EXPECT_CALL(mock_obserser, OnPrefSet(key1));
+ EXPECT_CALL(mock_obserser, OnPrefDeleted(_)).Times(0);
+ prefs_.SetString(key1, "value");
+ testing::Mock::VerifyAndClearExpectations(&mock_obserser);
+
+ EXPECT_CALL(mock_obserser, OnPrefSet(_)).Times(0);
+ EXPECT_CALL(mock_obserser, OnPrefDeleted(Eq(key1)));
+ prefs_.Delete(key1);
+ testing::Mock::VerifyAndClearExpectations(&mock_obserser);
+
prefs_.RemoveObserver(kKey, &mock_obserser);
}
@@ -359,6 +423,11 @@
EXPECT_TRUE(prefs_.Delete(kKey));
EXPECT_FALSE(prefs_.Exists(kKey));
EXPECT_TRUE(prefs_.Delete(kKey));
+
+ auto key = prefs_.CreateSubKey("ns", "sp", "sk");
+ ASSERT_TRUE(prefs_.SetInt64(key, 0));
+ EXPECT_TRUE(prefs_.Exists(key));
+ EXPECT_TRUE(prefs_.Delete(kKey));
}
} // namespace chromeos_update_engine
diff --git a/omaha_request_action.cc b/omaha_request_action.cc
index 81abb3e..c9b8aa0 100644
--- a/omaha_request_action.cc
+++ b/omaha_request_action.cc
@@ -413,37 +413,33 @@
continue;
const OmahaRequestParams::AppParams& dlc_params = it->second;
-
+ const string& dlc_id = dlc_params.name;
// Skip if the ping for this DLC was not sent.
if (!dlc_params.send_ping)
continue;
- base::FilePath metadata_path =
- base::FilePath(params_->dlc_prefs_root()).Append(dlc_params.name);
-
- Prefs prefs;
- if (!base::CreateDirectory(metadata_path) || !prefs.Init(metadata_path)) {
- LOG(ERROR) << "Failed to initialize the preferences path:"
- << metadata_path.value() << ".";
- continue;
- }
+ PrefsInterface* prefs = system_state_->prefs();
// Reset the active metadata value to |kPingInactiveValue|.
- if (!prefs.SetInt64(kPrefsPingActive, kPingInactiveValue))
- LOG(ERROR) << "Failed to set the value of ping metadata '"
- << kPrefsPingActive << "'.";
+ auto active_key =
+ prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ if (!prefs->SetInt64(active_key, kPingInactiveValue))
+ LOG(ERROR) << "Failed to set the value of ping metadata '" << active_key
+ << "'.";
- if (!prefs.SetString(kPrefsPingLastRollcall,
- parser_data.daystart_elapsed_days))
+ auto last_rollcall_key =
+ 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 '"
- << kPrefsPingLastRollcall << "'.";
+ << last_rollcall_key << "'.";
if (dlc_params.ping_active) {
// Write the value of elapsed_days into |kPrefsPingLastActive| only if
// the previous ping was an active one.
- if (!prefs.SetString(kPrefsPingLastActive,
- parser_data.daystart_elapsed_days))
+ auto last_active_key =
+ 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 '"
- << kPrefsPingLastActive << "'.";
+ << last_active_key << "'.";
}
}
}
diff --git a/omaha_request_action_unittest.cc b/omaha_request_action_unittest.cc
index 2528f7b..e1f5ef9 100644
--- a/omaha_request_action_unittest.cc
+++ b/omaha_request_action_unittest.cc
@@ -429,12 +429,6 @@
bool expected_allow_p2p_for_sharing,
const string& expected_p2p_url);
- // Helper function used to test the Ping request.
- // Create the test directory and setup the Omaha response.
- void SetUpStorePingReply(const string& dlc_id,
- base::FilePath* metadata_path_dlc,
- base::ScopedTempDir* tempdir);
-
FakeSystemState fake_system_state_;
FakeUpdateResponse fake_update_response_;
// Used by all tests.
@@ -453,6 +447,36 @@
string post_str;
};
+class OmahaRequestActionDlcPingTest : public OmahaRequestActionTest {
+ protected:
+ void SetUp() override {
+ OmahaRequestActionTest::SetUp();
+ dlc_id_ = "dlc0";
+ active_key_ = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id_, kPrefsPingActive);
+ last_active_key_ = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id_, kPrefsPingLastActive);
+ last_rollcall_key_ = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id_, kPrefsPingLastRollcall);
+
+ tuc_params_.http_response =
+ "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
+ "protocol=\"3.0\"><daystart elapsed_days=\"4763\" "
+ "elapsed_seconds=\"36540\"/><app appid=\"test-app-id\" status=\"ok\">\""
+ "<updatecheck status=\"noupdate\"/></app><app "
+ "appid=\"test-app-id_dlc0\" "
+ "status=\"ok\"><ping status=\"ok\"/><updatecheck status=\"noupdate\"/>"
+ "</app></response>";
+ tuc_params_.expected_check_result =
+ metrics::CheckResult::kNoUpdateAvailable;
+ tuc_params_.expected_check_reaction = metrics::CheckReaction::kUnset;
+ }
+
+ std::string dlc_id_;
+ std::string active_key_;
+ std::string last_active_key_;
+ std::string last_rollcall_key_;
+};
bool OmahaRequestActionTest::TestUpdateCheck() {
brillo::FakeMessageLoop loop(nullptr);
loop.SetAsCurrent();
@@ -2904,106 +2928,67 @@
EXPECT_EQ(kEolDateInvalid, StringToEolDate(eol_date));
}
-void OmahaRequestActionTest::SetUpStorePingReply(
- const string& dlc_id,
- base::FilePath* metadata_path_dlc,
- base::ScopedTempDir* tempdir) {
- // Create a uniquely named test directory.
- ASSERT_TRUE(tempdir->CreateUniqueTempDir());
- request_params_.set_root(tempdir->GetPath().value());
- *metadata_path_dlc =
- base::FilePath(request_params_.dlc_prefs_root()).Append(dlc_id);
- ASSERT_TRUE(base::CreateDirectory(*metadata_path_dlc));
-
- tuc_params_.http_response =
- "<?xml version=\"1.0\" encoding=\"UTF-8\"?><response "
- "protocol=\"3.0\"><daystart elapsed_days=\"4763\" "
- "elapsed_seconds=\"36540\"/><app appid=\"test-app-id\" status=\"ok\">\""
- "<updatecheck status=\"noupdate\"/></app><app appid=\"test-app-id_dlc0\" "
- "status=\"ok\"><ping status=\"ok\"/><updatecheck status=\"noupdate\"/>"
- "</app></response>";
- tuc_params_.expected_check_result = metrics::CheckResult::kNoUpdateAvailable;
- tuc_params_.expected_check_reaction = metrics::CheckReaction::kUnset;
-}
-
-TEST_F(OmahaRequestActionTest, StorePingReplyNoPing) {
- string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0;
- base::ScopedTempDir tempdir;
- SetUpStorePingReply(dlc_id, &metadata_path_dlc0, &tempdir);
- int64_t temp_int;
- Prefs prefs;
- ASSERT_TRUE(prefs.Init(metadata_path_dlc0));
-
- OmahaRequestParams::AppParams app_param = {.name = dlc_id};
+TEST_F(OmahaRequestActionDlcPingTest, StorePingReplyNoPing) {
+ OmahaRequestParams::AppParams app_param = {.name = dlc_id_};
request_params_.set_dlc_apps_params(
- {{request_params_.GetDlcAppId(dlc_id), app_param}});
+ {{request_params_.GetDlcAppId(dlc_id_), app_param}});
ASSERT_TRUE(TestUpdateCheck());
+
+ int64_t temp_int;
// If there was no ping, the metadata files shouldn't exist yet.
- EXPECT_FALSE(prefs.GetInt64(kPrefsPingActive, &temp_int));
- EXPECT_FALSE(prefs.GetInt64(kPrefsPingLastActive, &temp_int));
- EXPECT_FALSE(prefs.GetInt64(kPrefsPingLastRollcall, &temp_int));
+ EXPECT_FALSE(fake_prefs_.GetInt64(active_key_, &temp_int));
+ EXPECT_FALSE(fake_prefs_.GetInt64(last_active_key_, &temp_int));
+ EXPECT_FALSE(fake_prefs_.GetInt64(last_rollcall_key_, &temp_int));
}
-TEST_F(OmahaRequestActionTest, StorePingReplyActiveTest) {
- string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0;
- base::ScopedTempDir tempdir;
- SetUpStorePingReply(dlc_id, &metadata_path_dlc0, &tempdir);
- int64_t temp_int;
- Prefs prefs;
- ASSERT_TRUE(prefs.Init(metadata_path_dlc0));
+TEST_F(OmahaRequestActionDlcPingTest, StorePingReplyActiveTest) {
// Create Active value
- prefs.SetInt64(kPrefsPingActive, 0);
+ fake_prefs_.SetInt64(active_key_, 0);
OmahaRequestParams::AppParams app_param = {
.active_counting_type = OmahaRequestParams::kDateBased,
- .name = dlc_id,
+ .name = dlc_id_,
.ping_active = 1,
.send_ping = true};
request_params_.set_dlc_apps_params(
- {{request_params_.GetDlcAppId(dlc_id), app_param}});
+ {{request_params_.GetDlcAppId(dlc_id_), app_param}});
+ int64_t temp_int;
+ string temp_str;
ASSERT_TRUE(TestUpdateCheck());
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingActive, &temp_int));
+ EXPECT_TRUE(fake_prefs_.GetInt64(active_key_, &temp_int));
EXPECT_EQ(temp_int, kPingInactiveValue);
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingLastActive, &temp_int));
- EXPECT_EQ(temp_int, 4763);
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingLastRollcall, &temp_int));
- EXPECT_EQ(temp_int, 4763);
+ EXPECT_TRUE(fake_prefs_.GetString(last_active_key_, &temp_str));
+ EXPECT_EQ(temp_str, "4763");
+ EXPECT_TRUE(fake_prefs_.GetString(last_rollcall_key_, &temp_str));
+ EXPECT_EQ(temp_str, "4763");
}
-TEST_F(OmahaRequestActionTest, StorePingReplyInactiveTest) {
- string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0;
- base::ScopedTempDir tempdir;
- SetUpStorePingReply(dlc_id, &metadata_path_dlc0, &tempdir);
- int64_t temp_int;
- Prefs prefs;
- ASSERT_TRUE(prefs.Init(metadata_path_dlc0));
+TEST_F(OmahaRequestActionDlcPingTest, StorePingReplyInactiveTest) {
// Create Active value
- prefs.SetInt64(kPrefsPingActive, 0);
+ fake_prefs_.SetInt64(active_key_, 0);
OmahaRequestParams::AppParams app_param = {
.active_counting_type = OmahaRequestParams::kDateBased,
- .name = dlc_id,
+ .name = dlc_id_,
.ping_active = 0,
.send_ping = true};
request_params_.set_dlc_apps_params(
- {{request_params_.GetDlcAppId(dlc_id), app_param}});
+ {{request_params_.GetDlcAppId(dlc_id_), app_param}});
// Set the previous active value to an older value than 4763.
- prefs.SetInt64(kPrefsPingLastActive, 555);
+ fake_prefs_.SetString(last_active_key_, "555");
+ int64_t temp_int;
ASSERT_TRUE(TestUpdateCheck());
- ASSERT_TRUE(prefs.Init(metadata_path_dlc0));
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingActive, &temp_int));
+ EXPECT_TRUE(fake_prefs_.GetInt64(active_key_, &temp_int));
EXPECT_EQ(temp_int, kPingInactiveValue);
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingLastActive, &temp_int));
- EXPECT_EQ(temp_int, 555);
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingLastRollcall, &temp_int));
- EXPECT_EQ(temp_int, 4763);
+ string temp_str;
+ EXPECT_TRUE(fake_prefs_.GetString(last_active_key_, &temp_str));
+ EXPECT_EQ(temp_str, "555");
+ EXPECT_TRUE(fake_prefs_.GetString(last_rollcall_key_, &temp_str));
+ EXPECT_EQ(temp_str, "4763");
}
} // namespace chromeos_update_engine
diff --git a/omaha_request_params.cc b/omaha_request_params.cc
index 5267598..d4b8d64 100644
--- a/omaha_request_params.cc
+++ b/omaha_request_params.cc
@@ -214,7 +214,6 @@
void OmahaRequestParams::set_root(const string& root) {
root_ = root;
test::SetImagePropertiesRootPrefix(root_.c_str());
- dlc_prefs_root_ = root + kDlcMetadataRootpath;
}
int OmahaRequestParams::GetChannelIndex(const string& channel) const {
diff --git a/omaha_request_params.h b/omaha_request_params.h
index b33d0b1..d29ce70 100644
--- a/omaha_request_params.h
+++ b/omaha_request_params.h
@@ -58,7 +58,6 @@
update_check_count_wait_enabled_(false),
min_update_checks_needed_(kDefaultMinUpdateChecks),
max_update_checks_allowed_(kDefaultMaxUpdateChecks),
- dlc_prefs_root_(kDlcMetadataRootpath),
is_install_(false) {}
virtual ~OmahaRequestParams();
@@ -222,8 +221,6 @@
return autoupdate_token_;
}
- inline std::string dlc_prefs_root() const { return dlc_prefs_root_; }
-
// Returns the App ID corresponding to the current value of the
// download channel.
virtual std::string GetAppId() const;
@@ -410,9 +407,6 @@
// When reading files, prepend root_ to the paths. Useful for testing.
std::string root_;
- // The metadata/prefs root path for DLCs.
- std::string dlc_prefs_root_;
-
// A list of DLC modules to install. A mapping from DLC App ID to |AppParams|.
std::map<std::string, AppParams> dlc_apps_params_;
diff --git a/update_attempter.cc b/update_attempter.cc
index ae7f71e..0ead18a 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -28,7 +28,6 @@
#include <base/bind.h>
#include <base/compiler_specific.h>
-#include <base/files/file_enumerator.h>
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/rand_util.h>
@@ -85,6 +84,7 @@
using chromeos_update_manager::Policy;
using chromeos_update_manager::StagingCase;
using chromeos_update_manager::UpdateCheckParams;
+using std::map;
using std::string;
using std::vector;
using update_engine::UpdateAttemptFlags;
@@ -658,6 +658,22 @@
}
}
+bool UpdateAttempter::ResetDlcPrefs(const string& dlc_id) {
+ vector<string> failures;
+ PrefsInterface* prefs = system_state_->prefs();
+ for (auto& sub_key :
+ {kPrefsPingActive, kPrefsPingLastActive, kPrefsPingLastRollcall}) {
+ auto key = prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, sub_key);
+ if (!prefs->Delete(key))
+ failures.emplace_back(sub_key);
+ }
+ if (failures.size() != 0)
+ PLOG(ERROR) << "Failed to delete prefs (" << base::JoinString(failures, ",")
+ << " for DLC (" << dlc_id << ").";
+
+ return failures.size() == 0;
+}
+
bool UpdateAttempter::SetDlcActiveValue(bool is_active, const string& dlc_id) {
if (dlc_id.empty()) {
LOG(ERROR) << "Empty DLC ID passed.";
@@ -665,50 +681,30 @@
}
LOG(INFO) << "Set DLC (" << dlc_id << ") to "
<< (is_active ? "Active" : "Inactive");
- // TODO(andrewlassalle): Should dlc_prefs_root be in systemstate instead of
- // omaha_request_params_?
- base::FilePath metadata_path =
- base::FilePath(omaha_request_params_->dlc_prefs_root()).Append(dlc_id);
+ PrefsInterface* prefs = system_state_->prefs();
if (is_active) {
- base::File::Error error;
- if (!base::CreateDirectoryAndGetError(metadata_path, &error)) {
- PLOG(ERROR) << "Failed to create metadata directory for DLC (" << dlc_id
- << "). Error:" << error;
- return false;
- }
-
- Prefs prefs;
- if (!prefs.Init(metadata_path)) {
- LOG(ERROR) << "Failed to initialize the preferences path:"
- << metadata_path.value() << ".";
- return false;
- }
-
- if (!prefs.SetInt64(kPrefsPingActive, kPingActiveValue)) {
+ auto ping_active_key =
+ prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ if (!prefs->SetInt64(ping_active_key, kPingActiveValue)) {
LOG(ERROR) << "Failed to set the value of ping metadata '"
<< kPrefsPingActive << "'.";
return false;
}
} else {
- if (!base::DeleteFile(metadata_path, true)) {
- PLOG(ERROR) << "Failed to delete metadata directory("
- << metadata_path.value() << ") for DLC (" << dlc_id << ").";
- return false;
- }
+ return ResetDlcPrefs(dlc_id);
}
return true;
}
-int64_t UpdateAttempter::GetPingMetadata(
- const PrefsInterface& prefs, const std::string& metadata_name) const {
+int64_t UpdateAttempter::GetPingMetadata(const string& metadata_key) const {
// The first time a ping is sent, the metadata files containing the values
// sent back by the server still don't exist. A value of -1 is used to
// indicate this.
- if (!prefs.Exists(metadata_name))
+ if (!system_state_->prefs()->Exists(metadata_key))
return kPingNeverPinged;
int64_t value;
- if (prefs.GetInt64(metadata_name, &value))
+ if (system_state_->prefs()->GetInt64(metadata_key, &value))
return value;
// Return -2 when the file exists and there is a problem reading from it, or
@@ -724,49 +720,41 @@
LOG(INFO) << "Failed to retrieve DLC module IDs from dlcservice. Check the "
"state of dlcservice, will not update DLC modules.";
}
- base::FilePath metadata_root_path =
- base::FilePath(omaha_request_params_->dlc_prefs_root());
- // Cleanup any leftover metadata for DLCs which don't exist.
- base::FileEnumerator dir_enum(metadata_root_path,
- false /* recursive */,
- base::FileEnumerator::DIRECTORIES);
- std::unordered_set<string> dlc_ids(dlc_ids_.begin(), dlc_ids_.end());
- for (base::FilePath name = dir_enum.Next(); !name.empty();
- name = dir_enum.Next()) {
- string id = name.BaseName().value();
- if (dlc_ids.find(id) == dlc_ids.end()) {
- LOG(INFO) << "Deleting stale metadata for DLC:" << id;
- if (!base::DeleteFile(name, true))
- PLOG(WARNING) << "Failed to delete DLC prefs path:" << name.value();
- }
- }
- std::map<std::string, OmahaRequestParams::AppParams> dlc_apps_params;
+ PrefsInterface* prefs = system_state_->prefs();
+ map<string, OmahaRequestParams::AppParams> dlc_apps_params;
for (const auto& dlc_id : dlc_ids_) {
OmahaRequestParams::AppParams dlc_params{
.active_counting_type = OmahaRequestParams::kDateBased,
.name = dlc_id,
.send_ping = false};
- // Only send the ping when the request is to update DLCs. When installing
- // DLCs, we don't want to send the ping yet, since the DLCs might fail to
- // install or might not really be active yet.
- if (!is_install_) {
- base::FilePath metadata_path = metadata_root_path.Append(dlc_id);
- Prefs prefs;
- if (!base::CreateDirectory(metadata_path) || !prefs.Init(metadata_path)) {
- LOG(ERROR) << "Failed to initialize the preferences path:"
- << metadata_path.value() << ".";
- } else {
- dlc_params.ping_active = kPingActiveValue;
- if (!prefs.GetInt64(kPrefsPingActive, &dlc_params.ping_active) ||
- dlc_params.ping_active != kPingActiveValue) {
- dlc_params.ping_active = kPingInactiveValue;
- }
- dlc_params.ping_date_last_active =
- GetPingMetadata(prefs, kPrefsPingLastActive);
- dlc_params.ping_date_last_rollcall =
- GetPingMetadata(prefs, kPrefsPingLastRollcall);
- dlc_params.send_ping = true;
+ if (is_install_) {
+ // In some cases, |SetDlcActiveValue| might fail to reset the DLC prefs
+ // when a DLC is uninstalled. To avoid having stale values from that
+ // scenario, we reset the metadata values on a new install request.
+ // Ignore failure to delete stale prefs.
+ ResetDlcPrefs(dlc_id);
+ SetDlcActiveValue(true, dlc_id);
+ } else {
+ // Only send the ping when the request is to update DLCs. When installing
+ // DLCs, we don't want to send the ping yet, since the DLCs might fail to
+ // install or might not really be active yet.
+ dlc_params.ping_active = kPingActiveValue;
+ auto ping_active_key =
+ 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);
+ dlc_params.ping_date_last_active = GetPingMetadata(ping_last_active_key);
+
+ auto ping_last_rollcall_key =
+ prefs->CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ dlc_params.ping_date_last_rollcall =
+ GetPingMetadata(ping_last_rollcall_key);
+
+ dlc_params.send_ping = true;
}
dlc_apps_params[omaha_request_params_->GetDlcAppId(dlc_id)] = dlc_params;
}
diff --git a/update_attempter.h b/update_attempter.h
index 9e48179..e270b59 100644
--- a/update_attempter.h
+++ b/update_attempter.h
@@ -439,13 +439,15 @@
// Resets interactivity and forced update flags.
void ResetInteractivityFlags();
- // Get the integer values from the metadata directory set in |prefs| for
- // |kPrefsPingLastActive| or |kPrefsPingLastRollcall|.
+ // Resets all the DLC prefs.
+ bool ResetDlcPrefs(const std::string& dlc_id);
+
+ // Get the integer values from the DLC metadata for |kPrefsPingLastActive|
+ // or |kPrefsPingLastRollcall|.
// The value is equal to -2 when the value cannot be read or is not numeric.
// The value is equal to -1 the first time it is being sent, which is
// when the metadata file doesn't exist.
- int64_t GetPingMetadata(const PrefsInterface& prefs,
- const std::string& metadata_name) const;
+ int64_t GetPingMetadata(const std::string& metadata_key) const;
// Calculates the update parameters for DLCs. Sets the |dlc_ids_|
// parameter on the |omaha_request_params_| object.
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 56665ad..5a6a23e 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -72,6 +72,7 @@
using std::unordered_set;
using std::vector;
using testing::_;
+using testing::Contains;
using testing::DoAll;
using testing::ElementsAre;
using testing::Field;
@@ -2349,16 +2350,9 @@
}
TEST_F(UpdateAttempterTest, CalculateDlcParamsInstallTest) {
- // Create a uniquely named test directory.
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
-
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
+ FakePrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
attempter_.is_install_ = true;
attempter_.dlc_ids_ = {dlc_id};
attempter_.CalculateDlcParams();
@@ -2371,22 +2365,18 @@
EXPECT_EQ(false, dlc_app_params.send_ping);
// When the DLC gets installed, a ping is not sent, therefore we don't store
// the values sent by Omaha.
- EXPECT_FALSE(
- base::PathExists(metadata_path_dlc0.Append(kPrefsPingLastActive)));
- EXPECT_FALSE(
- base::PathExists(metadata_path_dlc0.Append(kPrefsPingLastRollcall)));
+ auto last_active_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_active_key));
+ auto last_rollcall_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+ EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_rollcall_key));
}
TEST_F(UpdateAttempterTest, CalculateDlcParamsNoPrefFilesTest) {
- // Create a uniquely named test directory.
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
+ FakePrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
@@ -2407,23 +2397,23 @@
}
TEST_F(UpdateAttempterTest, CalculateDlcParamsNonParseableValuesTest) {
- // Create a uniquely named test directory.
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
+ MemoryPrefs prefs;
+ fake_system_state_.set_prefs(&prefs);
EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
// Write non numeric values in the metadata files.
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingActive), "z2yz", 4);
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingLastActive), "z2yz", 4);
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingLastRollcall), "z2yz", 4);
+ auto active_key =
+ PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ auto last_active_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ auto last_rollcall_key = PrefsInterface::CreateSubKey(
+ 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");
attempter_.is_install_ = false;
attempter_.CalculateDlcParams();
@@ -2440,23 +2430,24 @@
}
TEST_F(UpdateAttempterTest, CalculateDlcParamsValidValuesTest) {
- // Create a uniquely named test directory.
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
+ MemoryPrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
.WillOnce(
DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
// Write numeric values in the metadata files.
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingActive), "1", 1);
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingLastActive), "78", 2);
- base::WriteFile(metadata_path_dlc0.Append(kPrefsPingLastRollcall), "99", 2);
+ auto active_key =
+ PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ auto last_active_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ auto last_rollcall_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastRollcall);
+
+ fake_system_state_.prefs()->SetInt64(active_key, 1);
+ fake_system_state_.prefs()->SetInt64(last_active_key, 78);
+ fake_system_state_.prefs()->SetInt64(last_rollcall_key, 99);
attempter_.is_install_ = false;
attempter_.CalculateDlcParams();
@@ -2473,58 +2464,64 @@
}
TEST_F(UpdateAttempterTest, CalculateDlcParamsRemoveStaleMetadata) {
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc0));
- base::FilePath metadata_path_dlc_stale =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append("stale");
- ASSERT_TRUE(base::CreateDirectory(metadata_path_dlc_stale));
- EXPECT_CALL(mock_dlcservice_, GetDlcsToUpdate(_))
- .WillOnce(
- DoAll(SetArgPointee<0>(std::vector<string>({dlc_id})), Return(true)));
+ FakePrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
+ auto active_key =
+ PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, kPrefsPingActive);
+ auto last_active_key = PrefsInterface::CreateSubKey(
+ kDlcPrefsSubDir, dlc_id, kPrefsPingLastActive);
+ auto last_rollcall_key = PrefsInterface::CreateSubKey(
+ 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);
+ EXPECT_TRUE(fake_system_state_.prefs()->Exists(active_key));
+ EXPECT_TRUE(fake_system_state_.prefs()->Exists(last_active_key));
+ EXPECT_TRUE(fake_system_state_.prefs()->Exists(last_rollcall_key));
- attempter_.is_install_ = false;
+ attempter_.dlc_ids_ = {dlc_id};
+ attempter_.is_install_ = true;
attempter_.CalculateDlcParams();
- EXPECT_TRUE(base::PathExists(metadata_path_dlc0));
- EXPECT_FALSE(base::PathExists(metadata_path_dlc_stale));
+ EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_active_key));
+ EXPECT_FALSE(fake_system_state_.prefs()->Exists(last_rollcall_key));
+ // Active key is set on install.
+ EXPECT_TRUE(fake_system_state_.prefs()->Exists(active_key));
+ int64_t temp_int;
+ EXPECT_TRUE(fake_system_state_.prefs()->GetInt64(active_key, &temp_int));
+ EXPECT_EQ(temp_int, kPingActiveValue);
}
TEST_F(UpdateAttempterTest, SetDlcActiveValue) {
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
+ FakePrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
attempter_.SetDlcActiveValue(true, dlc_id);
- Prefs prefs;
- ASSERT_TRUE(base::PathExists(metadata_path_dlc0));
- ASSERT_TRUE(prefs.Init(metadata_path_dlc0));
int64_t temp_int;
- EXPECT_TRUE(prefs.GetInt64(kPrefsPingActive, &temp_int));
+ auto active_key =
+ 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);
}
TEST_F(UpdateAttempterTest, SetDlcInactive) {
- base::ScopedTempDir tempdir;
- ASSERT_TRUE(tempdir.CreateUniqueTempDir());
- fake_system_state_.request_params()->set_root(tempdir.GetPath().value());
string dlc_id = "dlc0";
- base::FilePath metadata_path_dlc0 =
- base::FilePath(fake_system_state_.request_params()->dlc_prefs_root())
- .Append(dlc_id);
- base::CreateDirectory(metadata_path_dlc0);
- EXPECT_TRUE(base::PathExists(metadata_path_dlc0));
+ MemoryPrefs fake_prefs;
+ fake_system_state_.set_prefs(&fake_prefs);
+ auto sub_keys = {
+ kPrefsPingActive, kPrefsPingLastActive, kPrefsPingLastRollcall};
+ for (auto& sub_key : sub_keys) {
+ 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);
- EXPECT_FALSE(base::PathExists(metadata_path_dlc0));
+ for (auto& sub_key : sub_keys) {
+ auto key = PrefsInterface::CreateSubKey(kDlcPrefsSubDir, dlc_id, sub_key);
+ EXPECT_FALSE(fake_system_state_.prefs()->Exists(key));
+ }
}
TEST_F(UpdateAttempterTest, GetSuccessfulDlcIds) {