From 978e712cf915649cb9c33945f2aeb3b15c675c83 Mon Sep 17 00:00:00 2001 From: Vitaly Buka Date: Fri, 4 Mar 2016 17:32:23 -0800 Subject: Implement local_discovery_enabled setting Implementation will reset entire privet::Manager component if setting was changed to false, and recreate component if it was changed into true. Additionally removing of HTTP callback was added. BUG: 27480269 Change-Id: Ieea91057fc0bdbd5f12c439b791250c9cf6c3741 Reviewed-on: https://weave-review.googlesource.com/2833 Reviewed-by: Alex Vakulenko --- examples/provider/avahi_client.cc | 5 +++- examples/provider/event_http_server.cc | 16 +++++++++--- examples/provider/event_http_server.h | 5 +++- include/weave/provider/http_server.h | 3 +++ include/weave/provider/test/mock_http_server.h | 2 ++ src/device_manager.cc | 36 +++++++++++++++++++------- src/device_manager.h | 15 ++++++----- src/privet/privet_manager.cc | 14 +++++++++- src/privet/privet_manager.h | 1 + src/weave_unittest.cc | 8 ++++++ 10 files changed, 83 insertions(+), 22 deletions(-) diff --git a/examples/provider/avahi_client.cc b/examples/provider/avahi_client.cc index 27fae10..ddd4630 100644 --- a/examples/provider/avahi_client.cc +++ b/examples/provider/avahi_client.cc @@ -75,10 +75,10 @@ void AvahiClient::PublishService(const std::string& service_type, service_type.c_str(), nullptr, txt_list.get()); CHECK_GE(ret, 0) << avahi_strerror(ret); } else { + avahi_entry_group_reset(group_.get()); prev_port_ = port; prev_type_ = service_type; - avahi_entry_group_reset(group_.get()); CHECK(avahi_entry_group_is_empty(group_.get())); ret = avahi_entry_group_add_service_strlst( @@ -91,6 +91,9 @@ void AvahiClient::PublishService(const std::string& service_type, } void AvahiClient::StopPublishing(const std::string& service_name) { + prev_port_ = 0; + prev_type_.clear(); + CHECK(group_); avahi_entry_group_reset(group_.get()); } diff --git a/examples/provider/event_http_server.cc b/examples/provider/event_http_server.cc index 48faab1..3d64d15 100644 --- a/examples/provider/event_http_server.cc +++ b/examples/provider/event_http_server.cc @@ -143,7 +143,7 @@ void HttpServerImpl::NotFound(evhtp_request_t* req) { void HttpServerImpl::ProcessRequest(evhtp_request_t* req) { std::unique_ptr request{new RequestImpl{EventPtr{req}}}; std::string path = request->GetPath(); - auto it = handlers_.find(path); + auto it = handlers_.find(std::make_pair(path, req->htp)); if (it != handlers_.end()) { return it->second.Run(std::move(request)); } @@ -157,17 +157,27 @@ void HttpServerImpl::ProcessRequestCallback(evhtp_request_t* req, void* arg) { void HttpServerImpl::AddHttpRequestHandler( const std::string& path, const RequestHandlerCallback& callback) { - handlers_.insert(std::make_pair(path, callback)); + handlers_[std::make_pair(path, httpd_.get())] = callback; evhtp_set_cb(httpd_.get(), path.c_str(), &ProcessRequestCallback, this); } void HttpServerImpl::AddHttpsRequestHandler( const std::string& path, const RequestHandlerCallback& callback) { - handlers_.insert(std::make_pair(path, callback)); + handlers_[std::make_pair(path, httpsd_.get())] = callback; evhtp_set_cb(httpsd_.get(), path.c_str(), &ProcessRequestCallback, this); } +void HttpServerImpl::RemoveHttpRequestHandler(const std::string& path) { + handlers_.erase(std::make_pair(path, httpd_.get())); + evhtp_set_cb(httpd_.get(), path.c_str(), nullptr, nullptr); +} + +void HttpServerImpl::RemoveHttpsRequestHandler(const std::string& path) { + handlers_.erase(std::make_pair(path, httpsd_.get())); + evhtp_set_cb(httpsd_.get(), path.c_str(), nullptr, nullptr); +} + void HttpServerImpl::ProcessReply(std::shared_ptr request, int status_code, const std::string& data, diff --git a/examples/provider/event_http_server.h b/examples/provider/event_http_server.h index 8bb5dd9..bf8e6d4 100644 --- a/examples/provider/event_http_server.h +++ b/examples/provider/event_http_server.h @@ -33,6 +33,8 @@ class HttpServerImpl : public provider::HttpServer { const RequestHandlerCallback& callback) override; void AddHttpsRequestHandler(const std::string& path_prefix, const RequestHandlerCallback& callback) override; + void RemoveHttpRequestHandler(const std::string& path) override; + void RemoveHttpsRequestHandler(const std::string& path) override; uint16_t GetHttpPort() const override; uint16_t GetHttpsPort() const override; base::TimeDelta GetRequestTimeout() const override; @@ -48,7 +50,8 @@ class HttpServerImpl : public provider::HttpServer { const std::string& mime_type); void NotFound(evhtp_request_t* req); - std::map handlers_; + std::map, RequestHandlerCallback> + handlers_; std::vector cert_fingerprint_; EventTaskRunner* task_runner_{nullptr}; diff --git a/include/weave/provider/http_server.h b/include/weave/provider/http_server.h index 1c28d63..33aff35 100644 --- a/include/weave/provider/http_server.h +++ b/include/weave/provider/http_server.h @@ -132,6 +132,9 @@ class HttpServer { const std::string& path, const RequestHandlerCallback& callback) = 0; + virtual void RemoveHttpRequestHandler(const std::string& path) = 0; + virtual void RemoveHttpsRequestHandler(const std::string& path) = 0; + virtual uint16_t GetHttpPort() const = 0; virtual uint16_t GetHttpsPort() const = 0; virtual std::vector GetHttpsCertificateFingerprint() const = 0; diff --git a/include/weave/provider/test/mock_http_server.h b/include/weave/provider/test/mock_http_server.h index 995a8d8..e6b1f55 100644 --- a/include/weave/provider/test/mock_http_server.h +++ b/include/weave/provider/test/mock_http_server.h @@ -22,6 +22,8 @@ class MockHttpServer : public HttpServer { void(const std::string&, const RequestHandlerCallback&)); MOCK_METHOD2(AddHttpsRequestHandler, void(const std::string&, const RequestHandlerCallback&)); + MOCK_METHOD1(RemoveHttpRequestHandler, void(const std::string&)); + MOCK_METHOD1(RemoveHttpsRequestHandler, void(const std::string&)); MOCK_CONST_METHOD0(GetHttpPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsPort, uint16_t()); MOCK_CONST_METHOD0(GetHttpsCertificateFingerprint, std::vector()); diff --git a/src/device_manager.cc b/src/device_manager.cc index b6b91a9..3f8e4d7 100644 --- a/src/device_manager.cc +++ b/src/device_manager.cc @@ -30,7 +30,12 @@ DeviceManager::DeviceManager(provider::ConfigStore* config_store, provider::HttpServer* http_server, provider::Wifi* wifi, provider::Bluetooth* bluetooth) - : config_{new Config{config_store}}, + : task_runner_{task_runner}, + network_{network}, + dns_sd_{dns_sd}, + http_server_{http_server}, + wifi_{wifi}, + config_{new Config{config_store}}, component_manager_{new ComponentManagerImpl{task_runner}} { if (http_server) { access_revocation_manager_.reset( @@ -50,7 +55,9 @@ DeviceManager::DeviceManager(provider::ConfigStore* config_store, device_info_->Start(); if (http_server) { - StartPrivet(task_runner, network, dns_sd, http_server, wifi, bluetooth); + StartPrivet(); + AddSettingsChangedCallback(base::Bind(&DeviceManager::OnSettingsChanged, + weak_ptr_factory_.GetWeakPtr())); } else { CHECK(!dns_sd); } @@ -71,17 +78,18 @@ Config* DeviceManager::GetConfig() { return device_info_->GetMutableConfig(); } -void DeviceManager::StartPrivet(provider::TaskRunner* task_runner, - provider::Network* network, - provider::DnsServiceDiscovery* dns_sd, - provider::HttpServer* http_server, - provider::Wifi* wifi, - provider::Bluetooth* bluetooth) { - privet_.reset(new privet::Manager{task_runner}); - privet_->Start(network, dns_sd, http_server, wifi, auth_manager_.get(), +void DeviceManager::StartPrivet() { + if (privet_) + return; + privet_.reset(new privet::Manager{task_runner_}); + privet_->Start(network_, dns_sd_, http_server_, wifi_, auth_manager_.get(), device_info_.get(), component_manager_.get()); } +void DeviceManager::StopPrivet() { + privet_.reset(); +} + GcdState DeviceManager::GetGcdState() const { return device_info_->GetGcdState(); } @@ -188,6 +196,14 @@ void DeviceManager::AddPairingChangedCallbacks( privet_->AddOnPairingChangedCallbacks(begin_callback, end_callback); } +void DeviceManager::OnSettingsChanged(const Settings& settings) { + if (settings.local_discovery_enabled && http_server_) { + StartPrivet(); + } else { + StopPrivet(); + } +} + std::unique_ptr Device::Create(provider::ConfigStore* config_store, provider::TaskRunner* task_runner, provider::HttpClient* http_client, diff --git a/src/device_manager.h b/src/device_manager.h index 545f293..c41a627 100644 --- a/src/device_manager.h +++ b/src/device_manager.h @@ -81,12 +81,15 @@ class DeviceManager final : public Device { Config* GetConfig(); private: - void StartPrivet(provider::TaskRunner* task_runner, - provider::Network* network, - provider::DnsServiceDiscovery* dns_sd, - provider::HttpServer* http_server, - provider::Wifi* wifi, - provider::Bluetooth* bluetooth); + void StartPrivet(); + void StopPrivet(); + void OnSettingsChanged(const Settings& settings); + + provider::TaskRunner* task_runner_{nullptr}; + provider::Network* network_{nullptr}; + provider::DnsServiceDiscovery* dns_sd_{nullptr}; + provider::HttpServer* http_server_{nullptr}; + provider::Wifi* wifi_{nullptr}; std::unique_ptr config_; std::unique_ptr auth_manager_; diff --git a/src/privet/privet_manager.cc b/src/privet/privet_manager.cc index 36223ba..1858168 100644 --- a/src/privet/privet_manager.cc +++ b/src/privet/privet_manager.cc @@ -40,7 +40,17 @@ using provider::Wifi; Manager::Manager(TaskRunner* task_runner) : task_runner_{task_runner} {} -Manager::~Manager() {} +Manager::~Manager() { + if (privet_handler_) { + for (const auto& path : privet_handler_->GetHttpsPaths()) { + http_server_->RemoveHttpsRequestHandler(path); + } + + for (const auto& path : privet_handler_->GetHttpPaths()) { + http_server_->RemoveHttpRequestHandler(path); + } + } +} void Manager::Start(Network* network, DnsServiceDiscovery* dns_sd, @@ -49,6 +59,8 @@ void Manager::Start(Network* network, AuthManager* auth_manager, DeviceRegistrationInfo* device, ComponentManager* component_manager) { + http_server_ = http_server; + CHECK(http_server_); CHECK(auth_manager); CHECK(device); diff --git a/src/privet/privet_manager.h b/src/privet/privet_manager.h index c91d72c..0d95f7a 100644 --- a/src/privet/privet_manager.h +++ b/src/privet/privet_manager.h @@ -78,6 +78,7 @@ class Manager { void OnConnectivityChanged(); provider::TaskRunner* task_runner_{nullptr}; + provider::HttpServer* http_server_{nullptr}; std::unique_ptr cloud_; std::unique_ptr device_; std::unique_ptr security_; diff --git a/src/weave_unittest.cc b/src/weave_unittest.cc index 3b28001..d92dace 100644 --- a/src/weave_unittest.cc +++ b/src/weave_unittest.cc @@ -263,6 +263,14 @@ class WeaveTest : public ::testing::Test { const provider::HttpServer::RequestHandlerCallback& cb) { https_handlers_[path_prefix] = cb; })); + EXPECT_CALL(http_server_, RemoveHttpRequestHandler(_)) + .WillRepeatedly(Invoke([this](const std::string& path_prefix) { + http_handlers_.erase(path_prefix); + })); + EXPECT_CALL(http_server_, RemoveHttpsRequestHandler(_)) + .WillRepeatedly(Invoke([this](const std::string& path_prefix) { + https_handlers_.erase(path_prefix); + })); } void InitDefaultExpectations() { -- cgit v1.2.3