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>

7 files changed, 78 insertions, 34 deletions

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 @@ const char kSettingsDir[] = "/var/lib/weave/";
 
 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 @@ bool FileConfigStore::LoadDefaults(Settings* settings) {
 }
 
 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 @@ class FileConfigStore : public provider::ConfigStore {
   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 @@ namespace provider {
 // 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 @@ namespace provider {
 // 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 @@ class ConfigStore {
 
   // 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 @@ class MockConfigStore : public ConfigStore {
           "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::Load() {
 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 @@ void Config::Save() {
   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, Defaults) {
 }
 
 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 @@ TEST_F(ConfigTest, LoadState) {
     "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 @@ TEST_F(ConfigTest, Setters) {
 
   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 @@ TEST_F(ConfigTest, Setters) {
           '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 @@ class WeaveTest : public ::testing::Test {
   }
 
   void InitConfigStore() {
-    EXPECT_CALL(config_store_, SaveSettings("")).WillRepeatedly(Return());
+    EXPECT_CALL(config_store_, SaveSettings("config", _))
+        .WillRepeatedly(Return());
   }
 
   void InitNetwork() {