Merge: Add |name| into LoadSettings/SaveSettings
Libweave needs to store more than one config file.
BUG:25776798
Reviewed-on: https://weave-review.googlesource.com/2198
Reviewed-by: Alex Vakulenko <avakulenko@google.com>
(cherry picked from commit 7ecdf959f10b62f192be867c280a7885626d6b85)
Change-Id: I00ce2ef4e7d272d1a7cfaf73d1802429d4f73831
Reviewed-on: https://weave-review.googlesource.com/2420
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/examples/provider/file_config_store.cc b/examples/provider/file_config_store.cc
index 6faa242..af887a7 100644
--- a/examples/provider/file_config_store.cc
+++ b/examples/provider/file_config_store.cc
@@ -19,9 +19,15 @@
FileConfigStore::FileConfigStore(bool disable_security,
const std::string& model_id)
- : disable_security_{disable_security},
- model_id_{model_id},
- settings_path_{"/var/lib/weave/weave_settings_" + model_id + ".json"} {}
+ : disable_security_{disable_security}, model_id_{model_id} {}
+
+std::string FileConfigStore::GetPath(const std::string& name) const {
+ std::string path{kSettingsDir};
+ path += path + "weave_settings_" + model_id_;
+ if (!name.empty())
+ path += "_" + name;
+ return path + ".json";
+}
bool FileConfigStore::LoadDefaults(Settings* settings) {
char host_name[HOST_NAME_MAX] = {};
@@ -55,16 +61,21 @@
}
std::string FileConfigStore::LoadSettings() {
- LOG(INFO) << "Loading settings from " << settings_path_;
- std::ifstream str(settings_path_);
+ return LoadSettings("");
+}
+
+std::string FileConfigStore::LoadSettings(const std::string& name) {
+ LOG(INFO) << "Loading settings from " << GetPath(name);
+ std::ifstream str(GetPath(name));
return std::string(std::istreambuf_iterator<char>(str),
std::istreambuf_iterator<char>());
}
-void FileConfigStore::SaveSettings(const std::string& settings) {
+void FileConfigStore::SaveSettings(const std::string& name,
+ const std::string& settings) {
CHECK(mkdir(kSettingsDir, S_IRWXU) == 0 || errno == EEXIST);
- LOG(INFO) << "Saving settings to " << settings_path_;
- std::ofstream str(settings_path_);
+ LOG(INFO) << "Saving settings to " << GetPath(name);
+ std::ofstream str(GetPath(name));
str << settings;
}
diff --git a/examples/provider/file_config_store.h b/examples/provider/file_config_store.h
index 578f940..214194e 100644
--- a/examples/provider/file_config_store.h
+++ b/examples/provider/file_config_store.h
@@ -19,13 +19,16 @@
FileConfigStore(bool disable_security, const std::string& model_id);
bool LoadDefaults(Settings* settings) override;
+ std::string LoadSettings(const std::string& name) override;
+ void SaveSettings(const std::string& name,
+ const std::string& settings) override;
+
std::string LoadSettings() override;
- void SaveSettings(const std::string& settings) override;
private:
+ std::string GetPath(const std::string& name) const;
const bool disable_security_;
const std::string model_id_;
- const std::string settings_path_;
};
} // namespace examples
diff --git a/include/weave/provider/config_store.h b/include/weave/provider/config_store.h
index 1b7988f..991d750 100644
--- a/include/weave/provider/config_store.h
+++ b/include/weave/provider/config_store.h
@@ -36,8 +36,8 @@
// Implementation of LoadSettings() method should load previously
// stored settings from the persistent storage (file, flash, etc).
// For example:
-// std::string FileConfigStore::LoadSettings() {
-// std::ifstream str("/var/lib/weave/weave_settings.json");
+// std::string FileConfigStore::LoadSettings(const std::string& name) {
+// std::ifstream str("/var/lib/weave/weave_" + name + ".json");
// return std::string(std::istreambuf_iterator<char>(str),
// std::istreambuf_iterator<char>());
// }
@@ -47,8 +47,9 @@
// Implementation of SaveSettings(...) method should store data in the
// persistent storage (file, flash, etc).
// For example:
-// void FileConfigStore::SaveSettings(const std::string& settings) {
-// std::ofstream str(kSettingsPath);
+// void FileConfigStore::SaveSettings(const std::string& name,
+// const std::string& settings) {
+// std::ofstream str("/var/lib/weave/weave_" + name + ".json");
// str << settings;
// }
// It is highly recommended to protected data using encryption with
@@ -67,12 +68,18 @@
// Returns settings saved by SaveSettings during last run of libweave.
// Implementation should return data as-is without parsing or modifications.
- virtual std::string LoadSettings() = 0;
+ // |name| is the name of settings blob. Could be used as filename.
+ virtual std::string LoadSettings(const std::string& name) = 0;
// Saves settings. Implementation should save data as-is without parsing or
// modifications. Data stored in settings can be sensitive, so it's highly
// recommended to protect data, e.g. using encryption.
- virtual void SaveSettings(const std::string& settings) = 0;
+ // |name| is the name of settings blob. Could be used as filename.
+ virtual void SaveSettings(const std::string& name,
+ const std::string& settings) = 0;
+
+ // Deprecated: only for migration of old configs to version with |name|.
+ virtual std::string LoadSettings() = 0;
protected:
virtual ~ConfigStore() {}
diff --git a/include/weave/provider/test/mock_config_store.h b/include/weave/provider/test/mock_config_store.h
index 3873251..cdae693 100644
--- a/include/weave/provider/test/mock_config_store.h
+++ b/include/weave/provider/test/mock_config_store.h
@@ -39,11 +39,13 @@
"version": 1,
"device_id": "TEST_DEVICE_ID"
})"));
- EXPECT_CALL(*this, SaveSettings(_)).WillRepeatedly(Return());
+ EXPECT_CALL(*this, LoadSettings("config")).WillRepeatedly(Return(""));
+ EXPECT_CALL(*this, SaveSettings("config", _)).WillRepeatedly(Return());
}
MOCK_METHOD1(LoadDefaults, bool(Settings*));
+ MOCK_METHOD1(LoadSettings, std::string(const std::string&));
+ MOCK_METHOD2(SaveSettings, void(const std::string&, const std::string&));
MOCK_METHOD0(LoadSettings, std::string());
- MOCK_METHOD1(SaveSettings, void(const std::string&));
};
} // namespace test
diff --git a/src/config.cc b/src/config.cc
index 76be205..cf564c8 100644
--- a/src/config.cc
+++ b/src/config.cc
@@ -21,6 +21,8 @@
namespace weave {
+const char kConfigName[] = "config";
+
namespace config_keys {
const char kVersion[] = "version";
@@ -139,9 +141,12 @@
void Config::Transaction::LoadState() {
if (!config_->config_store_)
return;
- std::string json_string = config_->config_store_->LoadSettings();
- if (json_string.empty())
- return;
+ std::string json_string = config_->config_store_->LoadSettings(kConfigName);
+ if (json_string.empty()) {
+ json_string = config_->config_store_->LoadSettings();
+ if (json_string.empty())
+ return;
+ }
auto value = base::JSONReader::Read(json_string);
base::DictionaryValue* dict = nullptr;
@@ -266,7 +271,7 @@
base::JSONWriter::WriteWithOptions(
dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json_string);
- config_store_->SaveSettings(json_string);
+ config_store_->SaveSettings(kConfigName, json_string);
}
Config::Transaction::~Transaction() {
diff --git a/src/config_unittest.cc b/src/config_unittest.cc
index 0367516..4517428 100644
--- a/src/config_unittest.cc
+++ b/src/config_unittest.cc
@@ -17,18 +17,20 @@
using testing::_;
using testing::Invoke;
using testing::Return;
+using testing::WithArgs;
namespace weave {
+const char kConfigName[] = "config";
+
class ConfigTest : public ::testing::Test {
protected:
void SetUp() override {
- EXPECT_CALL(*this, OnConfigChanged(_))
- .Times(1); // Called from AddOnChangedCallback
Reload();
}
void Reload() {
+ EXPECT_CALL(*this, OnConfigChanged(_)).Times(1);
config_.reset(new Config{&config_store_});
config_->AddOnChangedCallback(
base::Bind(&ConfigTest::OnConfigChanged, base::Unretained(this)));
@@ -86,31 +88,45 @@
}
TEST_F(ConfigTest, LoadStateV0) {
- EXPECT_CALL(config_store_, LoadSettings())
+ EXPECT_CALL(config_store_, LoadSettings(kConfigName))
.WillOnce(Return(R"({
"device_id": "state_device_id"
})"));
- EXPECT_CALL(*this, OnConfigChanged(_)).Times(1);
Reload();
EXPECT_EQ("state_device_id", GetSettings().cloud_id);
EXPECT_FALSE(GetSettings().device_id.empty());
EXPECT_NE(GetSettings().cloud_id, GetSettings().device_id);
- EXPECT_CALL(config_store_, LoadSettings())
+ EXPECT_CALL(config_store_, LoadSettings(kConfigName))
.WillOnce(Return(R"({
"device_id": "state_device_id",
"cloud_id": "state_cloud_id"
})"));
- EXPECT_CALL(*this, OnConfigChanged(_)).Times(1);
Reload();
EXPECT_EQ("state_cloud_id", GetSettings().cloud_id);
EXPECT_EQ("state_device_id", GetSettings().device_id);
}
+TEST_F(ConfigTest, LoadStateUnnamed) {
+ EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(""));
+
+ EXPECT_CALL(config_store_, LoadSettings()).Times(1);
+
+ Reload();
+}
+
+TEST_F(ConfigTest, LoadStateNamed) {
+ EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return("{}"));
+
+ EXPECT_CALL(config_store_, LoadSettings()).Times(0);
+
+ Reload();
+}
+
TEST_F(ConfigTest, LoadState) {
auto state = R"({
"version": 1,
@@ -133,9 +149,8 @@
"secret": "c3RhdGVfc2VjcmV0",
"service_url": "state_service_url"
})";
- EXPECT_CALL(config_store_, LoadSettings()).WillOnce(Return(state));
+ EXPECT_CALL(config_store_, LoadSettings(kConfigName)).WillOnce(Return(state));
- EXPECT_CALL(*this, OnConfigChanged(_)).Times(1);
Reload();
EXPECT_EQ("state_client_id", GetSettings().client_id);
@@ -243,8 +258,8 @@
EXPECT_CALL(*this, OnConfigChanged(_)).Times(1);
- EXPECT_CALL(config_store_, SaveSettings(_))
- .WillOnce(Invoke([](const std::string& json) {
+ EXPECT_CALL(config_store_, SaveSettings(kConfigName, _))
+ .WillOnce(WithArgs<1>(Invoke([](const std::string& json) {
auto expected = R"({
'version': 1,
'api_key': 'set_api_key',
@@ -267,7 +282,7 @@
'service_url': 'set_service_url'
})";
EXPECT_JSON_EQ(expected, *test::CreateValue(json));
- }));
+ })));
change.Commit();
}
diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc
index 5bef931..c22eaa1 100644
--- a/src/weave_unittest.cc
+++ b/src/weave_unittest.cc
@@ -205,7 +205,8 @@
}
void InitConfigStore() {
- EXPECT_CALL(config_store_, SaveSettings("")).WillRepeatedly(Return());
+ EXPECT_CALL(config_store_, SaveSettings("config", _))
+ .WillRepeatedly(Return());
}
void InitNetwork() {
