diff options
author | Vitaly Buka <vitalybuka@google.com> | 2016-01-15 14:48:54 -0800 |
---|---|---|
committer | Vitaly Buka <vitalybuka@google.com> | 2016-01-30 01:11:15 +0000 |
commit | ac18fcf3e15a74d9980ea6b09d2482a86d7fdf18 (patch) | |
tree | 6a7c5c15db74b6fa7e79482268de79d974d0a908 | |
parent | d1e6c4ffefd875409426388ec54cc122253a05bb (diff) | |
download | libweave-ac18fcf3e15a74d9980ea6b09d2482a86d7fdf18.tar.gz |
Merge: Add write callback into SaveSettings function
Saving critical settings needs confirmation.
When command alters device config, it should be set "Done" only after
settings are actually saved.
BUG:25776798
Reviewed-on: https://weave-review.googlesource.com/2199
Reviewed-by: Alex Vakulenko <avakulenko@google.com>
(cherry picked from commit 42e508f2559e019d2fcc8f88adfd184b7a6bc3a4)
Change-Id: I693e3c17b3f2f707c8df7af29eefd48362980bce
Reviewed-on: https://weave-review.googlesource.com/2421
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
-rw-r--r-- | examples/daemon/common/daemon.h | 11 | ||||
-rw-r--r-- | examples/provider/file_config_store.cc | 14 | ||||
-rw-r--r-- | examples/provider/file_config_store.h | 9 | ||||
-rw-r--r-- | include/weave/provider/config_store.h | 11 | ||||
-rw-r--r-- | include/weave/provider/test/mock_config_store.h | 12 | ||||
-rw-r--r-- | src/config.cc | 5 | ||||
-rw-r--r-- | src/config_unittest.cc | 12 | ||||
-rw-r--r-- | src/weave_unittest.cc | 9 |
8 files changed, 54 insertions, 29 deletions
diff --git a/examples/daemon/common/daemon.h b/examples/daemon/common/daemon.h index 4cccff3..6dc021d 100644 --- a/examples/daemon/common/daemon.h +++ b/examples/daemon/common/daemon.h @@ -69,10 +69,11 @@ class Daemon { }; Daemon(const Options& opts) - : config_store_{new weave::examples::FileConfigStore( - opts.disable_security_, - opts.model_id_)}, - task_runner_{new weave::examples::EventTaskRunner}, + : task_runner_{new weave::examples::EventTaskRunner}, + config_store_{ + new weave::examples::FileConfigStore(opts.disable_security_, + opts.model_id_, + task_runner_.get())}, http_client_{new weave::examples::CurlHttpClient(task_runner_.get())}, network_{new weave::examples::EventNetworkImpl(task_runner_.get())}, bluetooth_{new weave::examples::BluetoothImpl} { @@ -114,8 +115,8 @@ class Daemon { LOG(INFO) << "Device registered: " << device->GetSettings().cloud_id; } - std::unique_ptr<weave::examples::FileConfigStore> config_store_; std::unique_ptr<weave::examples::EventTaskRunner> task_runner_; + std::unique_ptr<weave::examples::FileConfigStore> config_store_; std::unique_ptr<weave::examples::CurlHttpClient> http_client_; std::unique_ptr<weave::examples::EventNetworkImpl> network_; std::unique_ptr<weave::examples::BluetoothImpl> bluetooth_; diff --git a/examples/provider/file_config_store.cc b/examples/provider/file_config_store.cc index af887a7..31efaa7 100644 --- a/examples/provider/file_config_store.cc +++ b/examples/provider/file_config_store.cc @@ -12,14 +12,19 @@ #include <string> #include <vector> +#include <base/bind.h> + namespace weave { namespace examples { const char kSettingsDir[] = "/var/lib/weave/"; FileConfigStore::FileConfigStore(bool disable_security, - const std::string& model_id) - : disable_security_{disable_security}, model_id_{model_id} {} + const std::string& model_id, + provider::TaskRunner* task_runner) + : disable_security_{disable_security}, + model_id_{model_id}, + task_runner_{task_runner} {} std::string FileConfigStore::GetPath(const std::string& name) const { std::string path{kSettingsDir}; @@ -72,11 +77,14 @@ std::string FileConfigStore::LoadSettings(const std::string& name) { } void FileConfigStore::SaveSettings(const std::string& name, - const std::string& settings) { + const std::string& settings, + const DoneCallback& callback) { CHECK(mkdir(kSettingsDir, S_IRWXU) == 0 || errno == EEXIST); LOG(INFO) << "Saving settings to " << GetPath(name); std::ofstream str(GetPath(name)); str << settings; + if (!callback.is_null()) + task_runner_->PostDelayedTask(FROM_HERE, base::Bind(callback, nullptr), {}); } } // namespace examples diff --git a/examples/provider/file_config_store.h b/examples/provider/file_config_store.h index 214194e..e7398d1 100644 --- a/examples/provider/file_config_store.h +++ b/examples/provider/file_config_store.h @@ -10,18 +10,22 @@ #include <vector> #include <weave/provider/config_store.h> +#include <weave/provider/task_runner.h> namespace weave { namespace examples { class FileConfigStore : public provider::ConfigStore { public: - FileConfigStore(bool disable_security, const std::string& model_id); + FileConfigStore(bool disable_security, + const std::string& model_id, + provider::TaskRunner* task_runner); bool LoadDefaults(Settings* settings) override; std::string LoadSettings(const std::string& name) override; void SaveSettings(const std::string& name, - const std::string& settings) override; + const std::string& settings, + const DoneCallback& callback) override; std::string LoadSettings() override; @@ -29,6 +33,7 @@ class FileConfigStore : public provider::ConfigStore { std::string GetPath(const std::string& name) const; const bool disable_security_; const std::string model_id_; + provider::TaskRunner* task_runner_{nullptr}; }; } // namespace examples diff --git a/include/weave/provider/config_store.h b/include/weave/provider/config_store.h index 991d750..128eccc 100644 --- a/include/weave/provider/config_store.h +++ b/include/weave/provider/config_store.h @@ -13,6 +13,7 @@ #include <base/callback.h> #include <base/time/time.h> #include <weave/enum_to_string.h> +#include <weave/error.h> #include <weave/settings.h> namespace weave { @@ -48,9 +49,13 @@ namespace provider { // persistent storage (file, flash, etc). // For example: // void FileConfigStore::SaveSettings(const std::string& name, -// const std::string& settings) { +// const std::string& settings, +// const DoneCallback& callback) { // std::ofstream str("/var/lib/weave/weave_" + name + ".json"); // str << settings; +// if (!callback.is_null()) +// task_runner_->PostDelayedTask(FROM_HERE, base::Bind(callback, nullptr), +// {}); // } // It is highly recommended to protected data using encryption with // hardware backed key. @@ -75,8 +80,10 @@ class ConfigStore { // modifications. Data stored in settings can be sensitive, so it's highly // recommended to protect data, e.g. using encryption. // |name| is the name of settings blob. Could be used as filename. + // Implementation must call or post callback virtual void SaveSettings(const std::string& name, - const std::string& settings) = 0; + const std::string& settings, + const DoneCallback& callback) = 0; // Deprecated: only for migration of old configs to version with |name|. virtual std::string LoadSettings() = 0; diff --git a/include/weave/provider/test/mock_config_store.h b/include/weave/provider/test/mock_config_store.h index cdae693..e6411d6 100644 --- a/include/weave/provider/test/mock_config_store.h +++ b/include/weave/provider/test/mock_config_store.h @@ -40,11 +40,19 @@ class MockConfigStore : public ConfigStore { "device_id": "TEST_DEVICE_ID" })")); EXPECT_CALL(*this, LoadSettings("config")).WillRepeatedly(Return("")); - EXPECT_CALL(*this, SaveSettings("config", _)).WillRepeatedly(Return()); + EXPECT_CALL(*this, SaveSettings("config", _, _)) + .WillRepeatedly(testing::WithArgs<1, 2>(testing::Invoke( + [](const std::string& json, const DoneCallback& callback) { + if (!callback.is_null()) + callback.Run(nullptr); + }))); } MOCK_METHOD1(LoadDefaults, bool(Settings*)); MOCK_METHOD1(LoadSettings, std::string(const std::string&)); - MOCK_METHOD2(SaveSettings, void(const std::string&, const std::string&)); + MOCK_METHOD3(SaveSettings, + void(const std::string&, + const std::string&, + const DoneCallback&)); MOCK_METHOD0(LoadSettings, std::string()); }; diff --git a/src/config.cc b/src/config.cc index cf564c8..44d20dd 100644 --- a/src/config.cc +++ b/src/config.cc @@ -18,6 +18,7 @@ #include "src/data_encoding.h" #include "src/privet/privet_types.h" #include "src/string_utils.h" +#include "src/bind_lambda.h" namespace weave { @@ -271,7 +272,9 @@ void Config::Save() { base::JSONWriter::WriteWithOptions( dict, base::JSONWriter::OPTIONS_PRETTY_PRINT, &json_string); - config_store_->SaveSettings(kConfigName, json_string); + config_store_->SaveSettings( + kConfigName, json_string, + base::Bind([](ErrorPtr error) { CHECK(!error); })); } Config::Transaction::~Transaction() { diff --git a/src/config_unittest.cc b/src/config_unittest.cc index 4517428..fbb558a 100644 --- a/src/config_unittest.cc +++ b/src/config_unittest.cc @@ -258,9 +258,10 @@ TEST_F(ConfigTest, Setters) { EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); - EXPECT_CALL(config_store_, SaveSettings(kConfigName, _)) - .WillOnce(WithArgs<1>(Invoke([](const std::string& json) { - auto expected = R"({ + EXPECT_CALL(config_store_, SaveSettings(kConfigName, _, _)) + .WillOnce(WithArgs<1, 2>( + Invoke([](const std::string& json, const DoneCallback& callback) { + auto expected = R"({ 'version': 1, 'api_key': 'set_api_key', 'client_id': 'set_client_id', @@ -281,8 +282,9 @@ TEST_F(ConfigTest, Setters) { 'secret': 'AQIDBAU=', 'service_url': 'set_service_url' })"; - EXPECT_JSON_EQ(expected, *test::CreateValue(json)); - }))); + EXPECT_JSON_EQ(expected, *test::CreateValue(json)); + callback.Run(nullptr); + }))); change.Commit(); } diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc index c22eaa1..ebc66cd 100644 --- a/src/weave_unittest.cc +++ b/src/weave_unittest.cc @@ -204,11 +204,6 @@ class WeaveTest : public ::testing::Test { }))); } - void InitConfigStore() { - EXPECT_CALL(config_store_, SaveSettings("config", _)) - .WillRepeatedly(Return()); - } - void InitNetwork() { EXPECT_CALL(network_, AddConnectionChangedCallback(_)) .WillRepeatedly(Invoke( @@ -268,7 +263,6 @@ class WeaveTest : public ::testing::Test { } void InitDefaultExpectations() { - InitConfigStore(); InitNetwork(); EXPECT_CALL(wifi_, StartAccessPoint(MatchesRegex("TEST_NAME.*prv"))) .WillOnce(Return()); @@ -361,13 +355,11 @@ TEST_F(WeaveTest, Mocks) { } TEST_F(WeaveTest, StartMinimal) { - InitConfigStore(); device_ = weave::Device::Create(&config_store_, &task_runner_, &http_client_, &network_, nullptr, nullptr, &wifi_, nullptr); } TEST_F(WeaveTest, StartNoWifi) { - InitConfigStore(); InitNetwork(); InitHttpServer(); InitDnsSd(); @@ -451,7 +443,6 @@ class WeaveWiFiSetupTest : public WeaveTest { void SetUp() override { WeaveTest::SetUp(); - InitConfigStore(); InitHttpServer(); InitNetwork(); InitDnsSd(); |