Move ownership of libweave config to DeviceManager It was not pretty that global config was owner by class responsible for cloud communications. Change-Id: I1476c610662a9773d38eb2fc64e1e2a5c769b63c Reviewed-on: https://weave-review.googlesource.com/1826 Reviewed-by: Alex Vakulenko <avakulenko@google.com>
diff --git a/src/base_api_handler_unittest.cc b/src/base_api_handler_unittest.cc index cdb6151..ca47626 100644 --- a/src/base_api_handler_unittest.cc +++ b/src/base_api_handler_unittest.cc
@@ -53,11 +53,10 @@ .WillRepeatedly(Invoke(&component_manager_, &ComponentManager::AddCommandHandler)); - std::unique_ptr<Config> config{new Config{&config_store_}}; - config->Load(); - dev_reg_.reset(new DeviceRegistrationInfo(&component_manager_, - std::move(config), nullptr, - &http_client_, nullptr, nullptr)); + config_.Load(); + dev_reg_.reset(new DeviceRegistrationInfo(&config_, &component_manager_, + nullptr, &http_client_, nullptr, + nullptr)); EXPECT_CALL(device_, GetSettings()) .WillRepeatedly(ReturnRef(dev_reg_->GetSettings())); @@ -91,6 +90,7 @@ } provider::test::MockConfigStore config_store_; + Config config_{&config_store_}; StrictMock<provider::test::MockHttpClient> http_client_; std::unique_ptr<DeviceRegistrationInfo> dev_reg_; ComponentManagerImpl component_manager_;
diff --git a/src/device_manager.cc b/src/device_manager.cc index 743f5bb..e99ca4d 100644 --- a/src/device_manager.cc +++ b/src/device_manager.cc
@@ -27,26 +27,25 @@ provider::DnsServiceDiscovery* dns_sd, provider::HttpServer* http_server, provider::Wifi* wifi, - provider::Bluetooth* bluetooth) { - component_manager_.reset(new ComponentManagerImpl); - - std::unique_ptr<Config> config{new Config{config_store}}; - config->Load(); + provider::Bluetooth* bluetooth) + : config_{new Config{config_store}}, + component_manager_{new ComponentManagerImpl} { + config_->Load(); if (http_server) { auth_manager_.reset( - new privet::AuthManager(config->GetSettings().secret, + new privet::AuthManager(config_->GetSettings().secret, http_server->GetHttpsCertificateFingerprint())); - if (auth_manager_->GetSecret() != config->GetSettings().secret) { + if (auth_manager_->GetSecret() != config_->GetSettings().secret) { // There is no Config::OnChangedCallback registered. - Config::Transaction transaction(config.get()); + Config::Transaction transaction(config_.get()); transaction.set_secret(auth_manager_->GetSecret()); } } device_info_.reset(new DeviceRegistrationInfo( - component_manager_.get(), std::move(config), task_runner, http_client, + config_.get(), component_manager_.get(), task_runner, http_client, network, auth_manager_.get())); base_api_handler_.reset(new BaseApiHandler{device_info_.get(), this});
diff --git a/src/device_manager.h b/src/device_manager.h index 9b03b05..bf641e2 100644 --- a/src/device_manager.h +++ b/src/device_manager.h
@@ -100,6 +100,7 @@ provider::Wifi* wifi, provider::Bluetooth* bluetooth); + std::unique_ptr<Config> config_; std::unique_ptr<privet::AuthManager> auth_manager_; std::unique_ptr<ComponentManager> component_manager_; std::unique_ptr<DeviceRegistrationInfo> device_info_;
diff --git a/src/device_registration_info.cc b/src/device_registration_info.cc index 52d2515..6bd42bd 100644 --- a/src/device_registration_info.cc +++ b/src/device_registration_info.cc
@@ -237,17 +237,16 @@ } // anonymous namespace DeviceRegistrationInfo::DeviceRegistrationInfo( + Config* config, ComponentManager* component_manager, - std::unique_ptr<Config> - config, provider::TaskRunner* task_runner, provider::HttpClient* http_client, provider::Network* network, privet::AuthManager* auth_manager) : http_client_{http_client}, task_runner_{task_runner}, + config_{config}, component_manager_{component_manager}, - config_{std::move(config)}, network_{network}, auth_manager_{auth_manager} { cloud_backoff_policy_.reset(new BackoffEntry::Policy{}); @@ -631,7 +630,7 @@ access_token_expiration_ = base::Time::Now() + base::TimeDelta::FromSeconds(expires_in); - Config::Transaction change{config_.get()}; + Config::Transaction change{config_}; change.set_cloud_id(cloud_id); change.set_robot_account(robot_account); change.set_refresh_token(refresh_token); @@ -807,7 +806,7 @@ void DeviceRegistrationInfo::UpdateDeviceInfo(const std::string& name, const std::string& description, const std::string& location) { - Config::Transaction change{config_.get()}; + Config::Transaction change{config_}; change.set_name(name); change.set_description(description); change.set_location(location); @@ -821,7 +820,7 @@ void DeviceRegistrationInfo::UpdateBaseConfig(AuthScope anonymous_access_role, bool local_discovery_enabled, bool local_pairing_enabled) { - Config::Transaction change(config_.get()); + Config::Transaction change(config_); change.set_local_anonymous_access_role(anonymous_access_role); change.set_local_discovery_enabled(local_discovery_enabled); change.set_local_pairing_enabled(local_pairing_enabled); @@ -839,7 +838,7 @@ "Unable to change config for registered device"); return false; } - Config::Transaction change{config_.get()}; + Config::Transaction change{config_}; change.set_client_id(client_id); change.set_client_secret(client_secret); change.set_api_key(api_key); @@ -1273,7 +1272,7 @@ connected_to_cloud_ = false; LOG(INFO) << "Device is unregistered from the cloud. Deleting credentials"; - Config::Transaction change{config_.get()}; + Config::Transaction change{config_}; // Keep cloud_id to switch to detect kInvalidCredentials after restart. change.set_robot_account(""); change.set_refresh_token("");
diff --git a/src/device_registration_info.h b/src/device_registration_info.h index b1b9293..5ba56a1 100644 --- a/src/device_registration_info.h +++ b/src/device_registration_info.h
@@ -57,9 +57,8 @@ base::Callback<void(const base::DictionaryValue& response, ErrorPtr error)>; - DeviceRegistrationInfo(ComponentManager* component_manager, - std::unique_ptr<Config> - config, + DeviceRegistrationInfo(Config* config, + ComponentManager* component_manager, provider::TaskRunner* task_runner, provider::HttpClient* http_client, provider::Network* network, @@ -121,7 +120,7 @@ // TODO(vitalybuka): remove getters and pass config to dependent code. const Config::Settings& GetSettings() const { return config_->GetSettings(); } - Config* GetMutableConfig() { return config_.get(); } + Config* GetMutableConfig() { return config_; } GcdState GetGcdState() const { return gcd_state_; } @@ -306,11 +305,12 @@ provider::HttpClient* http_client_{nullptr}; provider::TaskRunner* task_runner_{nullptr}; + + Config* config_{nullptr}; + // Global component manager. ComponentManager* component_manager_{nullptr}; - std::unique_ptr<Config> config_; - // Backoff manager for DoCloudRequest() method. std::unique_ptr<BackoffEntry::Policy> cloud_backoff_policy_; std::unique_ptr<BackoffEntry> cloud_backoff_entry_;
diff --git a/src/device_registration_info_unittest.cc b/src/device_registration_info_unittest.cc index 029733e..d03ae15 100644 --- a/src/device_registration_info_unittest.cc +++ b/src/device_registration_info_unittest.cc
@@ -113,11 +113,9 @@ class DeviceRegistrationInfoTest : public ::testing::Test { protected: void SetUp() override { - std::unique_ptr<Config> config{new Config{&config_store_}}; - config_ = config.get(); - dev_reg_.reset(new DeviceRegistrationInfo{&component_manager_, - std::move(config), &task_runner_, - &http_client_, nullptr, nullptr}); + dev_reg_.reset(new DeviceRegistrationInfo{&config_, &component_manager_, + &task_runner_, &http_client_, + nullptr, nullptr}); ReloadDefaults(); } @@ -139,7 +137,7 @@ settings->service_url = test_data::kServiceURL; return true; })); - config_->Load(); + config_.Load(); dev_reg_->Start(); } @@ -184,7 +182,7 @@ provider::test::MockConfigStore config_store_; StrictMock<MockHttpClient> http_client_; base::DictionaryValue data_; - Config* config_{nullptr}; + Config config_{&config_store_}; std::unique_ptr<DeviceRegistrationInfo> dev_reg_; ComponentManagerImpl component_manager_; };