Implement a memory-based Prefs class.
When the /data storage is not available, we need to store the required
prefs in memory to keep all the code working. This patch splits the
Prefs class functionality and implements a MemoryPrefs class which
stores all the values in memory.
Bug: 27178350
TEST=Added unittest for the MemoryPrefs.
Change-Id: I11f871ddb73e2f33db4101705efb293e1cbe0023
diff --git a/common/prefs.cc b/common/prefs.cc
index a4b97d0..12d06c0 100644
--- a/common/prefs.cc
+++ b/common/prefs.cc
@@ -29,31 +29,12 @@
namespace chromeos_update_engine {
-bool Prefs::Init(const base::FilePath& prefs_dir) {
- prefs_dir_ = prefs_dir;
- return true;
+bool PrefsBase::GetString(const string& key, string* value) const {
+ return storage_->GetKey(key, value);
}
-bool Prefs::GetString(const string& key, string* value) const {
- base::FilePath filename;
- TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- if (!base::ReadFileToString(filename, value)) {
- LOG(INFO) << key << " not present in " << prefs_dir_.value();
- return false;
- }
- return true;
-}
-
-bool Prefs::SetString(const string& key, const string& value) {
- base::FilePath filename;
- TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- if (!base::DirectoryExists(filename.DirName())) {
- // Only attempt to create the directory if it doesn't exist to avoid calls
- // to parent directories where we might not have permission to write to.
- TEST_AND_RETURN_FALSE(base::CreateDirectory(filename.DirName()));
- }
- TEST_AND_RETURN_FALSE(base::WriteFile(filename, value.data(), value.size()) ==
- static_cast<int>(value.size()));
+bool PrefsBase::SetString(const string& key, const string& value) {
+ TEST_AND_RETURN_FALSE(storage_->SetKey(key, value));
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
std::vector<ObserverInterface*> copy_observers(observers_for_key->second);
@@ -63,7 +44,7 @@
return true;
}
-bool Prefs::GetInt64(const string& key, int64_t* value) const {
+bool PrefsBase::GetInt64(const string& key, int64_t* value) const {
string str_value;
if (!GetString(key, &str_value))
return false;
@@ -72,11 +53,11 @@
return true;
}
-bool Prefs::SetInt64(const string& key, const int64_t value) {
+bool PrefsBase::SetInt64(const string& key, const int64_t value) {
return SetString(key, base::Int64ToString(value));
}
-bool Prefs::GetBoolean(const string& key, bool* value) const {
+bool PrefsBase::GetBoolean(const string& key, bool* value) const {
string str_value;
if (!GetString(key, &str_value))
return false;
@@ -92,20 +73,16 @@
return false;
}
-bool Prefs::SetBoolean(const string& key, const bool value) {
+bool PrefsBase::SetBoolean(const string& key, const bool value) {
return SetString(key, value ? "true" : "false");
}
-bool Prefs::Exists(const string& key) const {
- base::FilePath filename;
- TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- return base::PathExists(filename);
+bool PrefsBase::Exists(const string& key) const {
+ return storage_->KeyExists(key);
}
-bool Prefs::Delete(const string& key) {
- base::FilePath filename;
- TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
- TEST_AND_RETURN_FALSE(base::DeleteFile(filename, false));
+bool PrefsBase::Delete(const string& key) {
+ TEST_AND_RETURN_FALSE(storage_->DeleteKey(key));
const auto observers_for_key = observers_.find(key);
if (observers_for_key != observers_.end()) {
std::vector<ObserverInterface*> copy_observers(observers_for_key->second);
@@ -115,11 +92,11 @@
return true;
}
-void Prefs::AddObserver(const string& key, ObserverInterface* observer) {
+void PrefsBase::AddObserver(const string& key, ObserverInterface* observer) {
observers_[key].push_back(observer);
}
-void Prefs::RemoveObserver(const string& key, ObserverInterface* observer) {
+void PrefsBase::RemoveObserver(const string& key, ObserverInterface* observer) {
std::vector<ObserverInterface*>& observers_for_key = observers_[key];
auto observer_it =
std::find(observers_for_key.begin(), observers_for_key.end(), observer);
@@ -127,8 +104,55 @@
observers_for_key.erase(observer_it);
}
-bool Prefs::GetFileNameForKey(const string& key,
- base::FilePath* filename) const {
+// Prefs
+
+bool Prefs::Init(const base::FilePath& prefs_dir) {
+ return file_storage_.Init(prefs_dir);
+}
+
+bool Prefs::FileStorage::Init(const base::FilePath& prefs_dir) {
+ prefs_dir_ = prefs_dir;
+ return true;
+}
+
+bool Prefs::FileStorage::GetKey(const string& key, string* value) const {
+ base::FilePath filename;
+ TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
+ if (!base::ReadFileToString(filename, value)) {
+ LOG(INFO) << key << " not present in " << prefs_dir_.value();
+ return false;
+ }
+ return true;
+}
+
+bool Prefs::FileStorage::SetKey(const string& key, const string& value) {
+ base::FilePath filename;
+ TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
+ if (!base::DirectoryExists(filename.DirName())) {
+ // Only attempt to create the directory if it doesn't exist to avoid calls
+ // to parent directories where we might not have permission to write to.
+ TEST_AND_RETURN_FALSE(base::CreateDirectory(filename.DirName()));
+ }
+ TEST_AND_RETURN_FALSE(base::WriteFile(filename, value.data(), value.size()) ==
+ static_cast<int>(value.size()));
+ return true;
+}
+
+bool Prefs::FileStorage::KeyExists(const string& key) const {
+ base::FilePath filename;
+ TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
+ return base::PathExists(filename);
+}
+
+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));
+ return true;
+}
+
+bool Prefs::FileStorage::GetFileNameForKey(const string& key,
+ base::FilePath* filename) const {
// Allows only non-empty keys containing [A-Za-z0-9_-].
TEST_AND_RETURN_FALSE(!key.empty());
for (size_t i = 0; i < key.size(); ++i) {
@@ -140,4 +164,33 @@
return true;
}
+// MemoryPrefs
+
+bool MemoryPrefs::MemoryStorage::GetKey(const string& key,
+ string* value) const {
+ auto it = values_.find(key);
+ if (it == values_.end())
+ return false;
+ *value = it->second;
+ return true;
+}
+
+bool MemoryPrefs::MemoryStorage::SetKey(const string& key,
+ const string& value) {
+ values_[key] = value;
+ return true;
+}
+
+bool MemoryPrefs::MemoryStorage::KeyExists(const string& key) const {
+ return values_.find(key) != values_.end();
+}
+
+bool MemoryPrefs::MemoryStorage::DeleteKey(const string& key) {
+ auto it = values_.find(key);
+ if (it == values_.end())
+ return false;
+ values_.erase(it);
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/common/prefs.h b/common/prefs.h
index f11abc3..0116454 100644
--- a/common/prefs.h
+++ b/common/prefs.h
@@ -28,18 +28,36 @@
namespace chromeos_update_engine {
-// Implements a preference store by storing the value associated with
-// a key in a separate file named after the key under a preference
-// store directory.
-
-class Prefs : public PrefsInterface {
+// Implements a preference store by storing the value associated with a key
+// in a given storage passed during construction.
+class PrefsBase : public PrefsInterface {
public:
- Prefs() = default;
+ // Storage interface used to set and retrieve keys.
+ class StorageInterface {
+ public:
+ StorageInterface() = default;
+ virtual ~StorageInterface() = default;
- // Initializes the store by associating this object with |prefs_dir|
- // as the preference store directory. Returns true on success, false
- // otherwise.
- bool Init(const base::FilePath& prefs_dir);
+ // Get the key named |key| and store its value in the referenced |value|.
+ // Returns whether the operation succeeded.
+ virtual bool GetKey(const std::string& key, std::string* value) const = 0;
+
+ // Set the value of the key named |key| to |value| regardless of the
+ // previous value. Returns whether the operation succeeded.
+ virtual bool SetKey(const std::string& key, const std::string& value) = 0;
+
+ // Returns whether the key named |key| exists.
+ virtual bool KeyExists(const std::string& key) const = 0;
+
+ // Deletes the value associated with the key name |key|. Returns whether the
+ // key was deleted.
+ virtual bool DeleteKey(const std::string& key) = 0;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(StorageInterface);
+ };
+
+ explicit PrefsBase(StorageInterface* storage) : storage_(storage) {}
// PrefsInterface methods.
bool GetString(const std::string& key, std::string* value) const override;
@@ -58,24 +76,93 @@
ObserverInterface* observer) override;
private:
+ // The registered observers watching for changes.
+ std::map<std::string, std::vector<ObserverInterface*>> observers_;
+
+ // The concrete implementation of the storage used for the keys.
+ StorageInterface* storage_;
+
+ DISALLOW_COPY_AND_ASSIGN(PrefsBase);
+};
+
+// Implements a preference store by storing the value associated with
+// a key in a separate file named after the key under a preference
+// store directory.
+
+class Prefs : public PrefsBase {
+ public:
+ Prefs() : PrefsBase(&file_storage_) {}
+
+ // Initializes the store by associating this object with |prefs_dir|
+ // as the preference store directory. Returns true on success, false
+ // otherwise.
+ bool Init(const base::FilePath& prefs_dir);
+
+ private:
FRIEND_TEST(PrefsTest, GetFileNameForKey);
FRIEND_TEST(PrefsTest, GetFileNameForKeyBadCharacter);
FRIEND_TEST(PrefsTest, GetFileNameForKeyEmpty);
- // Sets |filename| to the full path to the file containing the data
- // associated with |key|. Returns true on success, false otherwise.
- bool GetFileNameForKey(const std::string& key,
- base::FilePath* filename) const;
+ class FileStorage : public PrefsBase::StorageInterface {
+ public:
+ FileStorage() = default;
- // Preference store directory.
- base::FilePath prefs_dir_;
+ bool Init(const base::FilePath& prefs_dir);
- // The registered observers watching for changes.
- std::map<std::string, std::vector<ObserverInterface*>> observers_;
+ // PrefsBase::StorageInterface overrides.
+ bool GetKey(const std::string& key, std::string* value) const override;
+ bool SetKey(const std::string& key, const std::string& value) override;
+ bool KeyExists(const std::string& key) const override;
+ bool DeleteKey(const std::string& key) override;
+
+ private:
+ FRIEND_TEST(PrefsTest, GetFileNameForKey);
+ FRIEND_TEST(PrefsTest, GetFileNameForKeyBadCharacter);
+ FRIEND_TEST(PrefsTest, GetFileNameForKeyEmpty);
+
+ // Sets |filename| to the full path to the file containing the data
+ // associated with |key|. Returns true on success, false otherwise.
+ bool GetFileNameForKey(const std::string& key,
+ base::FilePath* filename) const;
+
+ // Preference store directory.
+ base::FilePath prefs_dir_;
+ };
+
+ // The concrete file storage implementation.
+ FileStorage file_storage_;
DISALLOW_COPY_AND_ASSIGN(Prefs);
};
+// Implements a preference store in memory. The stored values are lost when the
+// object is destroyed.
+
+class MemoryPrefs : public PrefsBase {
+ public:
+ MemoryPrefs() : PrefsBase(&mem_storage_) {}
+
+ private:
+ class MemoryStorage : public PrefsBase::StorageInterface {
+ public:
+ MemoryStorage() = default;
+
+ // PrefsBase::StorageInterface overrides.
+ bool GetKey(const std::string& key, std::string* value) const override;
+ bool SetKey(const std::string& key, const std::string& value) override;
+ bool KeyExists(const std::string& key) const override;
+ bool DeleteKey(const std::string& key) override;
+
+ private:
+ // The std::map holding the values in memory.
+ std::map<std::string, std::string> values_;
+ };
+
+ // The concrete memory storage implementation.
+ MemoryStorage mem_storage_;
+
+ DISALLOW_COPY_AND_ASSIGN(MemoryPrefs);
+};
} // namespace chromeos_update_engine
#endif // UPDATE_ENGINE_COMMON_PREFS_H_
diff --git a/common/prefs_unittest.cc b/common/prefs_unittest.cc
index 1000131..73ceb00 100644
--- a/common/prefs_unittest.cc
+++ b/common/prefs_unittest.cc
@@ -18,6 +18,7 @@
#include <inttypes.h>
+#include <limits>
#include <string>
#include <base/files/file_util.h>
@@ -61,18 +62,18 @@
const char kAllvalidCharsKey[] =
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz_-";
base::FilePath path;
- EXPECT_TRUE(prefs_.GetFileNameForKey(kAllvalidCharsKey, &path));
+ EXPECT_TRUE(prefs_.file_storage_.GetFileNameForKey(kAllvalidCharsKey, &path));
EXPECT_EQ(prefs_dir_.Append(kAllvalidCharsKey).value(), path.value());
}
TEST_F(PrefsTest, GetFileNameForKeyBadCharacter) {
base::FilePath path;
- EXPECT_FALSE(prefs_.GetFileNameForKey("ABC abc", &path));
+ EXPECT_FALSE(prefs_.file_storage_.GetFileNameForKey("ABC abc", &path));
}
TEST_F(PrefsTest, GetFileNameForKeyEmpty) {
base::FilePath path;
- EXPECT_FALSE(prefs_.GetFileNameForKey("", &path));
+ EXPECT_FALSE(prefs_.file_storage_.GetFileNameForKey("", &path));
}
TEST_F(PrefsTest, GetString) {
@@ -337,4 +338,24 @@
prefs_.RemoveObserver(kInvalidKey, &mock_obserser);
}
+class MemoryPrefsTest : public ::testing::Test {
+ protected:
+ MemoryPrefs prefs_;
+};
+
+TEST_F(MemoryPrefsTest, BasicTest) {
+ EXPECT_FALSE(prefs_.Exists(kKey));
+ int64_t value = 0;
+ EXPECT_FALSE(prefs_.GetInt64(kKey, &value));
+
+ EXPECT_TRUE(prefs_.SetInt64(kKey, 1234));
+ EXPECT_TRUE(prefs_.Exists(kKey));
+ EXPECT_TRUE(prefs_.GetInt64(kKey, &value));
+ EXPECT_EQ(1234, value);
+
+ EXPECT_TRUE(prefs_.Delete(kKey));
+ EXPECT_FALSE(prefs_.Exists(kKey));
+ EXPECT_FALSE(prefs_.Delete(kKey));
+}
+
} // namespace chromeos_update_engine
