Hide Config::Load method Constructor is going to load data instead. This makes clear that ::Load is one time operations. Change-Id: Id7b2275544cc034d30737bea21b3d8cc68e4a8a9 Reviewed-on: https://weave-review.googlesource.com/2080 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/include/weave/provider/test/mock_config_store.h b/include/weave/provider/test/mock_config_store.h index 01e1ef9..3873251 100644 --- a/include/weave/provider/test/mock_config_store.h +++ b/include/weave/provider/test/mock_config_store.h
@@ -35,7 +35,10 @@ return true; })); EXPECT_CALL(*this, LoadSettings()) - .WillRepeatedly(Return(R"({"device_id": "TEST_DEVICE_ID"})")); + .WillRepeatedly(Return(R"({ + "version": 1, + "device_id": "TEST_DEVICE_ID" + })")); EXPECT_CALL(*this, SaveSettings(_)).WillRepeatedly(Return()); } MOCK_METHOD1(LoadDefaults, bool(Settings*));
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc index 6f8460b..8b0f0b2 100644 --- a/src/base_api_handler_unittest.cc +++ b/src/base_api_handler_unittest.cc
@@ -54,7 +54,6 @@ .WillRepeatedly( Invoke(&component_manager_, &ComponentManager::AddCommandHandler)); - config_.Load(); dev_reg_.reset(new DeviceRegistrationInfo(&config_, &component_manager_, nullptr, &http_client_, nullptr, nullptr));
diff --git a/src/config.cc b/src/config.cc index 442a87a..37e907c 100644 --- a/src/config.cc +++ b/src/config.cc
@@ -85,7 +85,9 @@ : EnumToStringMap(kRootClientTokenOwnerMap) {} Config::Config(provider::ConfigStore* config_store) - : settings_{CreateDefaultSettings()}, config_store_{config_store} {} + : settings_{CreateDefaultSettings()}, config_store_{config_store} { + Load(); +} void Config::AddOnChangedCallback(const OnChangedCallback& callback) { on_changed_.push_back(callback);
diff --git a/src/config.h b/src/config.h index f5a8a77..6dc0a07 100644 --- a/src/config.h +++ b/src/config.h
@@ -46,8 +46,6 @@ void AddOnChangedCallback(const OnChangedCallback& callback); const Config::Settings& GetSettings() const; - void Load(); - // Allows editing of config. Makes sure that callbacks were called and changes // were saved. // User can commit changes by calling Commit method or by destroying the @@ -120,6 +118,7 @@ }; private: + void Load(); void Save(); Settings settings_;
diff --git a/src/config_unittest.cc b/src/config_unittest.cc index 1491d7b..0367516 100644 --- a/src/config_unittest.cc +++ b/src/config_unittest.cc
@@ -25,12 +25,16 @@ void SetUp() override { EXPECT_CALL(*this, OnConfigChanged(_)) .Times(1); // Called from AddOnChangedCallback - config_.AddOnChangedCallback( - base::Bind(&ConfigTest::OnConfigChanged, base::Unretained(this))); - default_.Load(); + Reload(); } - const Config::Settings& GetSettings() const { return config_.GetSettings(); } + void Reload() { + config_.reset(new Config{&config_store_}); + config_->AddOnChangedCallback( + base::Bind(&ConfigTest::OnConfigChanged, base::Unretained(this))); + } + + const Config::Settings& GetSettings() const { return config_->GetSettings(); } const Config::Settings& GetDefaultSettings() const { return default_.GetSettings(); @@ -39,7 +43,7 @@ MOCK_METHOD1(OnConfigChanged, void(const Settings&)); provider::test::MockConfigStore config_store_; - Config config_{&config_store_}; + std::unique_ptr<Config> config_{new Config{nullptr}}; Config default_{&config_store_}; }; @@ -50,6 +54,7 @@ } TEST_F(ConfigTest, Defaults) { + config_.reset(new Config{nullptr}); EXPECT_EQ("", GetSettings().client_id); EXPECT_EQ("", GetSettings().client_secret); EXPECT_EQ("", GetSettings().api_key); @@ -87,7 +92,7 @@ })")); EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); - config_.Load(); + Reload(); EXPECT_EQ("state_device_id", GetSettings().cloud_id); EXPECT_FALSE(GetSettings().device_id.empty()); @@ -100,7 +105,7 @@ })")); EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); - config_.Load(); + Reload(); EXPECT_EQ("state_cloud_id", GetSettings().cloud_id); EXPECT_EQ("state_device_id", GetSettings().device_id); @@ -131,7 +136,7 @@ EXPECT_CALL(config_store_, LoadSettings()).WillOnce(Return(state)); EXPECT_CALL(*this, OnConfigChanged(_)).Times(1); - config_.Load(); + Reload(); EXPECT_EQ("state_client_id", GetSettings().client_id); EXPECT_EQ("state_client_secret", GetSettings().client_secret); @@ -166,7 +171,7 @@ } TEST_F(ConfigTest, Setters) { - Config::Transaction change{&config_}; + Config::Transaction change{config_.get()}; change.set_client_id("set_client_id"); EXPECT_EQ("set_client_id", GetSettings().client_id);
diff --git a/src/device_manager.cc b/src/device_manager.cc index ab2e225..67a2c86 100644 --- a/src/device_manager.cc +++ b/src/device_manager.cc
@@ -30,8 +30,6 @@ provider::Bluetooth* bluetooth) : config_{new Config{config_store}}, component_manager_{new ComponentManagerImpl} { - config_->Load(); - if (http_server) { auth_manager_.reset(new privet::AuthManager( config_.get(), http_server->GetHttpsCertificateFingerprint()));
diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc index e21a147..ea81057 100644 --- a/src/device_registration_info_unittest.cc +++ b/src/device_registration_info_unittest.cc
@@ -126,10 +126,6 @@ void SetUp() override { EXPECT_CALL(clock_, Now()) .WillRepeatedly(Return(base::Time::FromTimeT(1450000000))); - dev_reg_.reset(new DeviceRegistrationInfo{&config_, &component_manager_, - &task_runner_, &http_client_, - nullptr, &auth_}); - ReloadDefaults(); } @@ -150,7 +146,10 @@ settings->service_url = test_data::kServiceURL; return true; })); - config_.Load(); + config_.reset(new Config{&config_store_}); + dev_reg_.reset(new DeviceRegistrationInfo{ + config_.get(), &component_manager_, &task_runner_, &http_client_, + nullptr, &auth_}); dev_reg_->Start(); } @@ -196,7 +195,7 @@ provider::test::MockConfigStore config_store_; StrictMock<MockHttpClient> http_client_; base::DictionaryValue data_; - Config config_{&config_store_}; + std::unique_ptr<Config> config_; test::MockClock clock_; privet::AuthManager auth_{ {68, 52, 36, 95, 74, 89, 25, 2, 31, 5, 65, 87, 64, 32, 17, 26, 8, 73, 57,