Async RegisterDevice implementation This allows to remove HttpClient::SendRequestAndBlock. BUG:24267885 Change-Id: I9e2bea86108d3a683be98121d88c5ce97a6af91c Reviewed-on: https://weave-review.googlesource.com/1292 Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/examples/ubuntu/curl_http_client.cc b/libweave/examples/ubuntu/curl_http_client.cc index 1b1e21f..1d69ce5 100644 --- a/libweave/examples/ubuntu/curl_http_client.cc +++ b/libweave/examples/ubuntu/curl_http_client.cc
@@ -16,9 +16,9 @@ struct ResponseImpl : public provider::HttpClient::Response { int GetStatusCode() const override { return status; } std::string GetContentType() const override { return content_type; } - const std::string& GetData() const override { return data; } + std::string GetData() const override { return data; } - int status; + long status{0}; std::string content_type; std::string data; };
diff --git a/libweave/examples/ubuntu/curl_http_client.h b/libweave/examples/ubuntu/curl_http_client.h index 7d62b3d..8d41c36 100644 --- a/libweave/examples/ubuntu/curl_http_client.h +++ b/libweave/examples/ubuntu/curl_http_client.h
@@ -24,11 +24,6 @@ public: explicit CurlHttpClient(provider::TaskRunner* task_runner); - std::unique_ptr<Response> SendRequestAndBlock(const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) override; void SendRequest(const std::string& method, const std::string& url, const Headers& headers, @@ -37,6 +32,12 @@ const ErrorCallback& error_callback) override; private: + std::unique_ptr<Response> SendRequestAndBlock(const std::string& method, + const std::string& url, + const Headers& headers, + const std::string& data, + ErrorPtr* error); + void RunSuccessCallback(const SuccessCallback& success_callback, std::unique_ptr<Response> response); void RunErrorCallback(const ErrorCallback& error_callback,
diff --git a/libweave/examples/ubuntu/event_http_client.cc b/libweave/examples/ubuntu/event_http_client.cc index f81916c..742eef8 100644 --- a/libweave/examples/ubuntu/event_http_client.cc +++ b/libweave/examples/ubuntu/event_http_client.cc
@@ -49,7 +49,7 @@ public: int GetStatusCode() const override { return status; } std::string GetContentType() const override { return content_type; } - const std::string& GetData() const { return data; } + std::string GetData() const { return data; } int status; std::string content_type; @@ -61,7 +61,7 @@ std::unique_ptr<evhttp_uri, EventDeleter> http_uri_; std::unique_ptr<evhttp_connection, EventDeleter> evcon_; HttpClient::SuccessCallback success_callback_; - HttpClient::ErrorCallback error_callback_; + ErrorCallback error_callback_; }; void RequestDoneCallback(evhttp_request* req, void* ctx) { @@ -93,15 +93,6 @@ EventHttpClient::EventHttpClient(EventTaskRunner* task_runner) : task_runner_{task_runner} {} -std::unique_ptr<provider::HttpClient::Response> -EventHttpClient::SendRequestAndBlock(const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) { - return nullptr; -} - void EventHttpClient::SendRequest(const std::string& method, const std::string& url, const Headers& headers,
diff --git a/libweave/examples/ubuntu/event_http_client.h b/libweave/examples/ubuntu/event_http_client.h index c38ad2e..53a01ab 100644 --- a/libweave/examples/ubuntu/event_http_client.h +++ b/libweave/examples/ubuntu/event_http_client.h
@@ -20,11 +20,6 @@ public: explicit EventHttpClient(EventTaskRunner* task_runner); - std::unique_ptr<Response> SendRequestAndBlock(const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) override; void SendRequest(const std::string& method, const std::string& url, const Headers& headers,
diff --git a/libweave/include/weave/provider/http_client.h b/libweave/include/weave/provider/http_client.h index cfc8937..995e7d6 100644 --- a/libweave/include/weave/provider/http_client.h +++ b/libweave/include/weave/provider/http_client.h
@@ -21,23 +21,13 @@ public: virtual int GetStatusCode() const = 0; virtual std::string GetContentType() const = 0; - virtual const std::string& GetData() const = 0; + virtual std::string GetData() const = 0; - // TODO(vitalybuka): Hide when SendRequestAndBlock is removed. virtual ~Response() = default; }; using Headers = std::vector<std::pair<std::string, std::string>>; using SuccessCallback = base::Callback<void(const Response&)>; - using ErrorCallback = base::Callback<void(const Error*)>; - - // TODO(vitalybuka): Remove blocking version. - virtual std::unique_ptr<Response> SendRequestAndBlock( - const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) = 0; virtual void SendRequest(const std::string& method, const std::string& url,
diff --git a/libweave/include/weave/provider/test/mock_http_client.h b/libweave/include/weave/provider/test/mock_http_client.h index 86aca46..ce3bfc3 100644 --- a/libweave/include/weave/provider/test/mock_http_client.h +++ b/libweave/include/weave/provider/test/mock_http_client.h
@@ -20,7 +20,7 @@ public: MOCK_CONST_METHOD0(GetStatusCode, int()); MOCK_CONST_METHOD0(GetContentType, std::string()); - MOCK_CONST_METHOD0(GetData, const std::string&()); + MOCK_CONST_METHOD0(GetData, std::string()); }; class MockHttpClient : public HttpClient { @@ -34,12 +34,6 @@ const std::string&, ErrorPtr*)); - std::unique_ptr<Response> SendRequestAndBlock(const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) override; - void SendRequest(const std::string& method, const std::string& url, const Headers& headers,
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc index 8975c19..7960b77 100644 --- a/libweave/src/device_registration_info.cc +++ b/libweave/src/device_registration_info.cc
@@ -118,14 +118,8 @@ HttpClient* transport) : method_{method}, url_{url}, transport_{transport} {} - std::unique_ptr<provider::HttpClient::Response> SendAndBlock( - ErrorPtr* error) { - return transport_->SendRequestAndBlock(method_, url_, GetFullHeaders(), - data_, error); - } - void Send(const HttpClient::SuccessCallback& success_callback, - const HttpClient::ErrorCallback& error_callback) { + const ErrorCallback& error_callback) { static int debug_id = 0; ++debug_id; VLOG(1) << "Sending request. id:" << debug_id << " method:" << method_ @@ -139,8 +133,7 @@ VLOG(2) << "Response data: " << response.GetData(); success_callback.Run(response); }; - auto on_error = [](int debug_id, - const HttpClient::ErrorCallback& error_callback, + auto on_error = [](int debug_id, const ErrorCallback& error_callback, const Error* error) { VLOG(1) << "Request failed, id=" << debug_id << ", reason: " << error->GetCode() @@ -543,22 +536,35 @@ error_callback); } +struct DeviceRegistrationInfo::RegisterCallbacks { + RegisterCallbacks(const SuccessCallback& success, const ErrorCallback& error) + : success_callback{success}, error_callback{error} {} + SuccessCallback success_callback; + ErrorCallback error_callback; +}; + +void DeviceRegistrationInfo::RegisterDeviceError( + const std::shared_ptr<RegisterCallbacks>& callbacks, + const Error* error) { + ErrorPtr error_clone = error->Clone(); + task_runner_->PostDelayedTask( + FROM_HERE, + base::Bind(callbacks->error_callback, base::Owned(error_clone.release())), + {}); +} + void DeviceRegistrationInfo::RegisterDevice( const std::string& ticket_id, const SuccessCallback& success_callback, const ErrorCallback& error_callback) { + auto callbacks = + std::make_shared<RegisterCallbacks>(success_callback, error_callback); + ErrorPtr error; - - auto on_error = [this, &error_callback](ErrorPtr error) { - task_runner_->PostDelayedTask( - FROM_HERE, base::Bind(error_callback, base::Owned(error.release())), - {}); - }; - std::unique_ptr<base::DictionaryValue> device_draft = BuildDeviceResource(&error); if (!device_draft) - return on_error(std::move(error)); + return RegisterDeviceError(callbacks, error.get()); base::DictionaryValue req_json; req_json.SetString("id", ticket_id); @@ -570,29 +576,46 @@ RequestSender sender{http::kPatch, url, http_client_}; sender.SetJsonData(req_json); - auto response = sender.SendAndBlock(&error); + sender.Send(base::Bind(&DeviceRegistrationInfo::RegisterDeviceOnTicketSent, + weak_factory_.GetWeakPtr(), ticket_id, callbacks), + base::Bind(&DeviceRegistrationInfo::RegisterDeviceError, + weak_factory_.GetWeakPtr(), callbacks)); +} - if (!response) - return on_error(std::move(error)); - auto json_resp = ParseJsonResponse(*response, &error); +void DeviceRegistrationInfo::RegisterDeviceOnTicketSent( + const std::string& ticket_id, + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response) { + ErrorPtr error; + auto json_resp = ParseJsonResponse(response, &error); if (!json_resp) - return on_error(std::move(error)); - if (!IsSuccessful(*response)) { + return RegisterDeviceError(callbacks, error.get()); + + if (!IsSuccessful(response)) { ParseGCDError(json_resp.get(), &error); - return on_error(std::move(error)); + return RegisterDeviceError(callbacks, error.get()); } - url = GetServiceURL("registrationTickets/" + ticket_id + "/finalize", - {{"key", GetSettings().api_key}}); - response = RequestSender{http::kPost, url, http_client_}.SendAndBlock(&error); - if (!response) - return on_error(std::move(error)); - json_resp = ParseJsonResponse(*response, &error); + std::string url = + GetServiceURL("registrationTickets/" + ticket_id + "/finalize", + {{"key", GetSettings().api_key}}); + RequestSender{http::kPost, url, http_client_}.Send( + base::Bind(&DeviceRegistrationInfo::RegisterDeviceOnTicketFinalized, + weak_factory_.GetWeakPtr(), callbacks), + base::Bind(&DeviceRegistrationInfo::RegisterDeviceError, + weak_factory_.GetWeakPtr(), callbacks)); +} + +void DeviceRegistrationInfo::RegisterDeviceOnTicketFinalized( + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response) { + ErrorPtr error; + auto json_resp = ParseJsonResponse(response, &error); if (!json_resp) - return on_error(std::move(error)); - if (!IsSuccessful(*response)) { + return RegisterDeviceError(callbacks, error.get()); + if (!IsSuccessful(response)) { ParseGCDError(json_resp.get(), &error); - return on_error(std::move(error)); + return RegisterDeviceError(callbacks, error.get()); } std::string auth_code; @@ -605,7 +628,7 @@ !device_draft_response->GetString("id", &cloud_id)) { Error::AddTo(&error, FROM_HERE, kErrorDomainGCD, "unexpected_response", "Device account missing in response"); - return on_error(std::move(error)); + return RegisterDeviceError(callbacks, error.get()); } UpdateDeviceInfoTimestamp(*device_draft_response); @@ -619,12 +642,20 @@ {"redirect_uri", "oob"}, {"scope", "https://www.googleapis.com/auth/clouddevices"}, {"grant_type", "authorization_code"}}); - response = sender2.SendAndBlock(&error); + sender2.Send(base::Bind(&DeviceRegistrationInfo::RegisterDeviceOnAuthCodeSent, + weak_factory_.GetWeakPtr(), cloud_id, robot_account, + callbacks), + base::Bind(&DeviceRegistrationInfo::RegisterDeviceError, + weak_factory_.GetWeakPtr(), callbacks)); +} - if (!response) - return on_error(std::move(error)); - - json_resp = ParseOAuthResponse(*response, &error); +void DeviceRegistrationInfo::RegisterDeviceOnAuthCodeSent( + const std::string& cloud_id, + const std::string& robot_account, + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response) { + ErrorPtr error; + auto json_resp = ParseOAuthResponse(response, &error); int expires_in = 0; std::string refresh_token; if (!json_resp || !json_resp->GetString("access_token", &access_token_) || @@ -633,7 +664,7 @@ access_token_.empty() || refresh_token.empty() || expires_in <= 0) { Error::AddTo(&error, FROM_HERE, kErrorDomainGCD, "unexpected_response", "Device access_token missing in response"); - return on_error(std::move(error)); + return RegisterDeviceError(callbacks, error.get()); } access_token_expiration_ = @@ -645,7 +676,7 @@ change.set_refresh_token(refresh_token); change.Commit(); - task_runner_->PostDelayedTask(FROM_HERE, success_callback, {}); + task_runner_->PostDelayedTask(FROM_HERE, callbacks->success_callback, {}); StartNotificationChannel();
diff --git a/libweave/src/device_registration_info.h b/libweave/src/device_registration_info.h index 74bf3a8..fdc4dcd 100644 --- a/libweave/src/device_registration_info.h +++ b/libweave/src/device_registration_info.h
@@ -275,6 +275,22 @@ // Wipes out the device registration information and stops server connections. void MarkDeviceUnregistered(); + class RegisterCallbacks; + void RegisterDeviceError(const std::shared_ptr<RegisterCallbacks>& callbacks, + const Error* error); + void RegisterDeviceOnTicketSent( + const std::string& ticket_id, + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response); + void RegisterDeviceOnTicketFinalized( + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response); + void RegisterDeviceOnAuthCodeSent( + const std::string& cloud_id, + const std::string& robot_account, + const std::shared_ptr<RegisterCallbacks>& callbacks, + const provider::HttpClient::Response& response); + // Transient data std::string access_token_; base::Time access_token_expiration_;
diff --git a/libweave/src/device_registration_info_unittest.cc b/libweave/src/device_registration_info_unittest.cc index 5245458..4a47013 100644 --- a/libweave/src/device_registration_info_unittest.cc +++ b/libweave/src/device_registration_info_unittest.cc
@@ -91,7 +91,7 @@ .WillRepeatedly(Return(http::kJsonUtf8)); EXPECT_CALL(*response, GetData()) .Times(AtLeast(1)) - .WillRepeatedly(ReturnRefOfCopy(text)); + .WillRepeatedly(Return(text)); return response; }
diff --git a/libweave/src/test/mock_http_client.cc b/libweave/src/test/mock_http_client.cc index 94cbe08..c3ffbef 100644 --- a/libweave/src/test/mock_http_client.cc +++ b/libweave/src/test/mock_http_client.cc
@@ -11,16 +11,6 @@ namespace provider { namespace test { -std::unique_ptr<HttpClient::Response> MockHttpClient::SendRequestAndBlock( - const std::string& method, - const std::string& url, - const Headers& headers, - const std::string& data, - ErrorPtr* error) { - return std::unique_ptr<Response>{ - MockSendRequest(method, url, headers, data, error)}; -} - void MockHttpClient::SendRequest(const std::string& method, const std::string& url, const Headers& headers,
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc index 69250fd..5334e6a 100644 --- a/libweave/src/weave_unittest.cc +++ b/libweave/src/weave_unittest.cc
@@ -151,7 +151,7 @@ .WillRepeatedly(Return("application/json; charset=utf-8")); EXPECT_CALL(*response, GetData()) .Times(AtLeast(1)) - .WillRepeatedly(ReturnRefOfCopy(json_response)); + .WillRepeatedly(Return(json_response)); return response; })); }