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,
