Prefs: Implement and use Prefs::GetBoolean/SetBoolean.
Adds a pair of functions GetBoolean/SetBoolean to the list Prefs
class to read and write persisted boolean values. The stored
values are not compatible with the SetInt64 and will return
false when attempting to get or set a value of the other type.
These functions are now used to store the update over cellular
user setting.
BUG=chromium:213401
TEST=sudo ./update_engine_unittests
Change-Id: I44107e33f8e81ae900671d9ba6a4f5779c2353db
Reviewed-on: https://gerrit.chromium.org/gerrit/61352
Reviewed-by: Joy Chen <[email protected]>
Tested-by: Alex Deymo <[email protected]>
Reviewed-by: mukesh agrawal <[email protected]>
Commit-Queue: Alex Deymo <[email protected]>
diff --git a/connection_manager.cc b/connection_manager.cc
index 185f3b5..b4bda0c 100644
--- a/connection_manager.cc
+++ b/connection_manager.cc
@@ -192,9 +192,11 @@
return false;
}
- int64_t stored_value;
- if (!prefs->GetInt64(kPrefsUpdateOverCellularPermission, &stored_value))
+ bool stored_value;
+ if (!prefs->GetBoolean(kPrefsUpdateOverCellularPermission,
+ &stored_value)) {
return false;
+ }
if (!stored_value) {
LOG(INFO) << "Disabling updates over cellular connection per user "
diff --git a/connection_manager_unittest.cc b/connection_manager_unittest.cc
index 724061b..5271ee1 100644
--- a/connection_manager_unittest.cc
+++ b/connection_manager_unittest.cc
@@ -306,18 +306,18 @@
EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
.Times(1)
.WillOnce(Return(true));
- EXPECT_CALL(*prefs, GetInt64(kPrefsUpdateOverCellularPermission, _))
+ EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<1>(1), Return(true)));
+ .WillOnce(DoAll(SetArgumentPointee<1>(true), Return(true)));
EXPECT_TRUE(cmut_.IsUpdateAllowedOver(kNetCellular));
// Block per user pref.
EXPECT_CALL(*prefs, Exists(kPrefsUpdateOverCellularPermission))
.Times(1)
.WillOnce(Return(true));
- EXPECT_CALL(*prefs, GetInt64(kPrefsUpdateOverCellularPermission, _))
+ EXPECT_CALL(*prefs, GetBoolean(kPrefsUpdateOverCellularPermission, _))
.Times(1)
- .WillOnce(DoAll(SetArgumentPointee<1>(0), Return(true)));
+ .WillOnce(DoAll(SetArgumentPointee<1>(false), Return(true)));
EXPECT_FALSE(cmut_.IsUpdateAllowedOver(kNetCellular));
}
diff --git a/dbus_service.cc b/dbus_service.cc
index 1a209cd..8619b2d 100644
--- a/dbus_service.cc
+++ b/dbus_service.cc
@@ -257,11 +257,11 @@
chromeos_update_engine::PrefsInterface* prefs = self->system_state_->prefs();
- if (!prefs->SetInt64(
+ if (!prefs->SetBoolean(
chromeos_update_engine::kPrefsUpdateOverCellularPermission,
- allowed ? 1 : 0)) {
+ allowed)) {
LOG(ERROR) << "Error setting the update over cellular to "
- << (allowed ? 1 : 0);
+ << (allowed ? "true" : "false");
*error = NULL;
return FALSE;
}
diff --git a/prefs.cc b/prefs.cc
index 496b59e..dda1919 100644
--- a/prefs.cc
+++ b/prefs.cc
@@ -53,6 +53,26 @@
return SetString(key, base::Int64ToString(value));
}
+bool Prefs::GetBoolean(const std::string& key, bool* value) {
+ string str_value;
+ if (!GetString(key, &str_value))
+ return false;
+ TrimWhitespaceASCII(str_value, TRIM_ALL, &str_value);
+ if (str_value == "false") {
+ *value = false;
+ return true;
+ }
+ if (str_value == "true") {
+ *value = true;
+ return true;
+ }
+ return false;
+}
+
+bool Prefs::SetBoolean(const std::string& key, const bool value) {
+ return SetString(key, value ? "true" : "false");
+}
+
bool Prefs::Exists(const string& key) {
FilePath filename;
TEST_AND_RETURN_FALSE(GetFileNameForKey(key, &filename));
diff --git a/prefs.h b/prefs.h
index e128eda..55f721f 100644
--- a/prefs.h
+++ b/prefs.h
@@ -29,6 +29,8 @@
bool SetString(const std::string& key, const std::string& value);
bool GetInt64(const std::string& key, int64_t* value);
bool SetInt64(const std::string& key, const int64_t value);
+ bool GetBoolean(const std::string& key, bool* value);
+ bool SetBoolean(const std::string& key, const bool value);
bool Exists(const std::string& key);
bool Delete(const std::string& key);
diff --git a/prefs_interface.h b/prefs_interface.h
index 9456d52..0e7ad7d 100644
--- a/prefs_interface.h
+++ b/prefs_interface.h
@@ -34,6 +34,15 @@
// false otherwise.
virtual bool SetInt64(const std::string& key, const int64_t value) = 0;
+ // Gets a boolean |value| associated with |key|. Returns true on
+ // success, false on failure (including when the |key| is not
+ // present in the store).
+ virtual bool GetBoolean(const std::string& key, bool* value) = 0;
+
+ // Associates |key| with a boolean |value|. Returns true on success,
+ // false otherwise.
+ virtual bool SetBoolean(const std::string& key, const bool value) = 0;
+
// Returns true if the setting exists (i.e. a file with the given key
// exists in the prefs directory)
virtual bool Exists(const std::string& key) = 0;
diff --git a/prefs_mock.h b/prefs_mock.h
index 4f991fc..a5492e2 100644
--- a/prefs_mock.h
+++ b/prefs_mock.h
@@ -19,6 +19,9 @@
MOCK_METHOD2(GetInt64, bool(const std::string& key, int64_t* value));
MOCK_METHOD2(SetInt64, bool(const std::string& key, const int64_t value));
+ MOCK_METHOD2(GetBoolean, bool(const std::string& key, bool* value));
+ MOCK_METHOD2(SetBoolean, bool(const std::string& key, const bool value));
+
MOCK_METHOD1(Exists, bool(const std::string& key));
MOCK_METHOD1(Delete, bool(const std::string& key));
};
diff --git a/prefs_unittest.cc b/prefs_unittest.cc
index 87c41b6..d2a07ee 100644
--- a/prefs_unittest.cc
+++ b/prefs_unittest.cc
@@ -187,6 +187,63 @@
EXPECT_EQ(StringPrintf("%" PRIi64, kint64min), value);
}
+TEST_F(PrefsTest, GetBooleanFalse) {
+ const char kKey[] = "test-key";
+ ASSERT_TRUE(SetValue(kKey, " \n false \t "));
+ bool value;
+ EXPECT_TRUE(prefs_.GetBoolean(kKey, &value));
+ EXPECT_FALSE(value);
+}
+
+TEST_F(PrefsTest, GetBooleanTrue) {
+ const char kKey[] = "test-key";
+ ASSERT_TRUE(SetValue(kKey, " \t true \n "));
+ bool value;
+ EXPECT_TRUE(prefs_.GetBoolean(kKey, &value));
+ EXPECT_TRUE(value);
+}
+
+TEST_F(PrefsTest, GetBooleanBadValue) {
+ const char kKey[] = "test-key";
+ ASSERT_TRUE(SetValue(kKey, "1"));
+ bool value;
+ EXPECT_FALSE(prefs_.GetBoolean(kKey, &value));
+}
+
+TEST_F(PrefsTest, GetBooleanBadEmptyValue) {
+ const char kKey[] = "test-key";
+ ASSERT_TRUE(SetValue(kKey, ""));
+ bool value;
+ EXPECT_FALSE(prefs_.GetBoolean(kKey, &value));
+}
+
+TEST_F(PrefsTest, GetBooleanNonExistentKey) {
+ bool value;
+ EXPECT_FALSE(prefs_.GetBoolean("random-key", &value));
+}
+
+TEST_F(PrefsTest, SetBooleanTrue) {
+ const char kKey[] = "test-bool";
+ EXPECT_TRUE(prefs_.SetBoolean(kKey, true));
+ string value;
+ EXPECT_TRUE(file_util::ReadFileToString(prefs_dir_.Append(kKey), &value));
+ EXPECT_EQ("true", value);
+}
+
+TEST_F(PrefsTest, SetBooleanFalse) {
+ const char kKey[] = "test-bool";
+ EXPECT_TRUE(prefs_.SetBoolean(kKey, false));
+ string value;
+ EXPECT_TRUE(file_util::ReadFileToString(prefs_dir_.Append(kKey), &value));
+ EXPECT_EQ("false", value);
+}
+
+TEST_F(PrefsTest, SetBooleanBadKey) {
+ const char kKey[] = "s p a c e s";
+ EXPECT_FALSE(prefs_.SetBoolean(kKey, true));
+ EXPECT_FALSE(file_util::PathExists(prefs_dir_.Append(kKey)));
+}
+
TEST_F(PrefsTest, ExistsWorks) {
const char kKey[] = "exists-key";
