buffet: Implement exponential backoff for cloud requests
Simplified logic of DoCloudRequest() method in buffet to de-couple
the multiple levels of callbacks and retries. Also added support for
exponential backoff on request failures.
In the process made RefreshAccessToken and GetDeviceInfo methods
fully asynchronous.
BUG=brillo:955
TEST=`FEATURES=test emerge-link buffet`
`test_that -b link 100.96.49.59 e:buffet_.*`
Change-Id: Ieeb2fa42ea25f15841bad5c6c09c6c9990f96943
Reviewed-on: https://chromium-review.googlesource.com/280833
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Tested-by: Alex Vakulenko <avakulenko@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
diff --git a/buffet/device_registration_info.cc b/buffet/device_registration_info.cc
index 279e2b9..00899ce 100644
--- a/buffet/device_registration_info.cc
+++ b/buffet/device_registration_info.cc
@@ -120,6 +120,17 @@
state_manager_{state_manager},
config_{std::move(config)},
notifications_enabled_{notifications_enabled} {
+ cloud_backoff_policy_.reset(new chromeos::BackoffEntry::Policy{});
+ cloud_backoff_policy_->num_errors_to_ignore = 0;
+ cloud_backoff_policy_->initial_delay_ms = 100;
+ cloud_backoff_policy_->multiply_factor = 2.0;
+ cloud_backoff_policy_->jitter_factor = 0.1;
+ cloud_backoff_policy_->maximum_backoff_ms = 30000;
+ cloud_backoff_policy_->entry_lifetime_ms = -1;
+ cloud_backoff_policy_->always_use_initial_delay = false;
+ cloud_backoff_entry_.reset(
+ new chromeos::BackoffEntry{cloud_backoff_policy_.get()});
+
command_manager_->AddOnCommandDefChanged(
base::Bind(&DeviceRegistrationInfo::OnCommandDefsChanged,
weak_factory_.GetWeakPtr()));
@@ -188,11 +199,6 @@
later);
}
-bool DeviceRegistrationInfo::CheckRegistration(chromeos::ErrorPtr* error) {
- return HaveRegistrationCredentials(error) &&
- MaybeRefreshAccessToken(error);
-}
-
bool DeviceRegistrationInfo::HaveRegistrationCredentials(
chromeos::ErrorPtr* error) {
const bool have_credentials = !config_->refresh_token().empty() &&
@@ -233,36 +239,48 @@
return resp;
}
-bool DeviceRegistrationInfo::MaybeRefreshAccessToken(
- chromeos::ErrorPtr* error) {
- LOG(INFO) << "Checking access token expiration.";
- if (!access_token_.empty() &&
- !access_token_expiration_.is_null() &&
- access_token_expiration_ > base::Time::Now()) {
- LOG(INFO) << "Access token is still valid.";
- return true;
- }
- return RefreshAccessToken(error);
+void DeviceRegistrationInfo::RefreshAccessToken(
+ const base::Closure& success_callback,
+ const CloudRequestErrorCallback& error_callback) {
+ LOG(INFO) << "Refreshing access token.";
+ // Make a shared pointer to |error_callback| since we are going to share this
+ // callback with both success and error callbacks for PostFormData() and if
+ // |error_callback| has any move-only types, one of the copies will be bad.
+ auto shared_error_callback =
+ std::make_shared<CloudRequestErrorCallback>(error_callback);
+
+ auto on_refresh_error = [shared_error_callback](
+ chromeos::http::RequestID id,
+ const chromeos::Error* error) {
+ shared_error_callback->Run(error);
+ };
+
+ chromeos::http::FormFieldList form_data{
+ {"refresh_token", config_->refresh_token()},
+ {"client_id", config_->client_id()},
+ {"client_secret", config_->client_secret()},
+ {"grant_type", "refresh_token"},
+ };
+
+ chromeos::http::PostFormData(
+ GetOAuthURL("token"), form_data, {}, transport_,
+ base::Bind(&DeviceRegistrationInfo::OnRefreshAccessTokenSuccess,
+ weak_factory_.GetWeakPtr(),
+ success_callback, shared_error_callback),
+ base::Bind(on_refresh_error));
}
-bool DeviceRegistrationInfo::RefreshAccessToken(
- chromeos::ErrorPtr* error) {
- LOG(INFO) << "Refreshing access token.";
- auto response = chromeos::http::PostFormDataAndBlock(
- GetOAuthURL("token"),
- {
- {"refresh_token", config_->refresh_token()},
- {"client_id", config_->client_id()},
- {"client_secret", config_->client_secret()},
- {"grant_type", "refresh_token"},
- },
- {}, transport_, error);
- if (!response)
- return false;
-
- auto json = ParseOAuthResponse(response.get(), error);
- if (!json)
- return false;
+void DeviceRegistrationInfo::OnRefreshAccessTokenSuccess(
+ const base::Closure& success_callback,
+ const std::shared_ptr<CloudRequestErrorCallback>& error_callback,
+ chromeos::http::RequestID id,
+ std::unique_ptr<chromeos::http::Response> response) {
+ chromeos::ErrorPtr error;
+ auto json = ParseOAuthResponse(response.get(), &error);
+ if (!json) {
+ error_callback->Run(error.get());
+ return;
+ }
int expires_in = 0;
if (!json->GetString("access_token", &access_token_) ||
@@ -270,10 +288,11 @@
access_token_.empty() ||
expires_in <= 0) {
LOG(ERROR) << "Access token unavailable.";
- chromeos::Error::AddTo(error, FROM_HERE, kErrorDomainOAuth2,
+ chromeos::Error::AddTo(&error, FROM_HERE, kErrorDomainOAuth2,
"unexpected_server_response",
"Access token unavailable");
- return false;
+ error_callback->Run(error.get());
+ return;
}
access_token_expiration_ = base::Time::Now() +
base::TimeDelta::FromSeconds(expires_in);
@@ -286,11 +305,7 @@
// Now that we have a new access token, retry the connection.
StartNotificationChannel();
}
- return true;
-}
-
-void DeviceRegistrationInfo::RunRefreshAccessToken() {
- RefreshAccessToken(nullptr);
+ success_callback.Run();
}
void DeviceRegistrationInfo::StartNotificationChannel() {
@@ -387,26 +402,11 @@
return resource;
}
-std::unique_ptr<base::DictionaryValue> DeviceRegistrationInfo::GetDeviceInfo(
- chromeos::ErrorPtr* error) {
- if (!CheckRegistration(error))
- return std::unique_ptr<base::DictionaryValue>();
-
- // TODO(antonm): Switch to DoCloudRequest later.
- auto response = chromeos::http::GetAndBlock(
- GetDeviceURL(), {GetAuthorizationHeader()}, transport_, error);
- int status_code = 0;
- std::unique_ptr<base::DictionaryValue> json =
- chromeos::http::ParseJsonResponse(response.get(), &status_code, error);
- if (json) {
- if (status_code >= chromeos::http::status_code::BadRequest) {
- LOG(WARNING) << "Failed to retrieve the device info. Response code = "
- << status_code;
- ParseGCDError(json.get(), error);
- return std::unique_ptr<base::DictionaryValue>();
- }
- }
- return json;
+void DeviceRegistrationInfo::GetDeviceInfo(
+ const CloudRequestCallback& success_callback,
+ const CloudRequestErrorCallback& error_callback) {
+ DoCloudRequest(chromeos::http::request_type::kGet, GetDeviceURL(), nullptr,
+ success_callback, error_callback);
}
std::string DeviceRegistrationInfo::RegisterDevice(const std::string& ticket_id,
@@ -502,145 +502,136 @@
return device_id;
}
-namespace {
-
-using ResponsePtr = std::unique_ptr<chromeos::http::Response>;
-
-void SendRequestWithRetries(
- const std::string& method,
- const std::string& url,
- const std::string& data,
- const std::string& mime_type,
- const chromeos::http::HeaderList& headers,
- std::shared_ptr<chromeos::http::Transport> transport,
- int num_retries,
- const chromeos::http::SuccessCallback& success_callback,
- const chromeos::http::ErrorCallback& error_callback) {
- auto on_failure =
- [method, url, data, mime_type, headers, transport, num_retries,
- success_callback, error_callback](int request_id,
- const chromeos::Error* error) {
- if (num_retries > 0) {
- SendRequestWithRetries(method, url, data, mime_type,
- headers, transport, num_retries - 1,
- success_callback, error_callback);
- } else {
- error_callback.Run(request_id, error);
- }
- };
-
- auto on_success =
- [on_failure, success_callback, error_callback](int request_id,
- ResponsePtr response) {
- int status_code = response->GetStatusCode();
- if (status_code >= chromeos::http::status_code::Continue &&
- status_code < chromeos::http::status_code::BadRequest) {
- success_callback.Run(request_id, std::move(response));
- return;
- }
-
- // TODO(antonm): Should add some useful information to error.
- LOG(WARNING) << "Request failed. Response code = " << status_code;
-
- chromeos::ErrorPtr error;
- chromeos::Error::AddTo(&error, FROM_HERE, chromeos::errors::http::kDomain,
- std::to_string(status_code),
- response->GetStatusText());
- if (status_code >= chromeos::http::status_code::InternalServerError &&
- status_code < 600) {
- // Request was valid, but server failed, retry.
- // TODO(antonm): Implement exponential backoff.
- // TODO(antonm): Reconsider status codes, maybe only some require
- // retry.
- // TODO(antonm): Support Retry-After header.
- on_failure(request_id, error.get());
- } else {
- error_callback.Run(request_id, error.get());
- }
- };
-
- chromeos::http::SendRequest(method, url, data.c_str(), data.size(),
- mime_type, headers, transport,
- base::Bind(on_success),
- base::Bind(on_failure));
-}
-
-} // namespace
-
void DeviceRegistrationInfo::DoCloudRequest(
const std::string& method,
const std::string& url,
const base::DictionaryValue* body,
const CloudRequestCallback& success_callback,
const CloudRequestErrorCallback& error_callback) {
+ // We make CloudRequestData shared here because we want to make sure
+ // there is only one instance of success_callback and error_calback since
+ // those may have move-only types and making a copy of the callback with
+ // move-only types curried-in will invalidate the source callback.
+ auto data = std::make_shared<CloudRequestData>();
+ data->method = method;
+ data->url = url;
+ if (body)
+ base::JSONWriter::Write(*body, &data->body);
+ data->success_callback = success_callback;
+ data->error_callback = error_callback;
+ SendCloudRequest(data);
+}
+
+void DeviceRegistrationInfo::SendCloudRequest(
+ const std::shared_ptr<const CloudRequestData>& data) {
// TODO(antonm): Add reauthorization on access token expiration (do not
// forget about 5xx when fetching new access token).
// TODO(antonm): Add support for device removal.
- std::string data;
- if (body)
- base::JSONWriter::Write(*body, &data);
+ VLOG(1) << "Sending cloud request '" << data->method << "' to '" << data->url
+ << "' with request body '" << data->body << "'";
+ chromeos::ErrorPtr error;
+ if (!HaveRegistrationCredentials(&error)) {
+ data->error_callback.Run(error.get());
+ return;
+ }
- const std::string mime_type{chromeos::mime::AppendParameter(
- chromeos::mime::application::kJson,
- chromeos::mime::parameters::kCharset,
- "utf-8")};
+ if (cloud_backoff_entry_->ShouldRejectRequest()) {
+ VLOG(1) << "Cloud request delayed for "
+ << cloud_backoff_entry_->GetTimeUntilRelease()
+ << " due to backoff policy";
+ base::MessageLoop::current()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&DeviceRegistrationInfo::SendCloudRequest, AsWeakPtr(),
+ data),
+ cloud_backoff_entry_->GetTimeUntilRelease());
+ return;
+ }
- auto status_cb = base::Bind(&DeviceRegistrationInfo::SetRegistrationStatus,
- weak_factory_.GetWeakPtr());
+ using chromeos::mime::application::kJson;
+ using chromeos::mime::parameters::kCharset;
+ const std::string mime_type =
+ chromeos::mime::AppendParameter(kJson, kCharset, "utf-8");
- auto request_cb = [success_callback, error_callback, status_cb](
- int request_id, ResponsePtr response) {
- status_cb.Run(RegistrationStatus::kConnected);
- chromeos::ErrorPtr error;
+ chromeos::http::RequestID request_id = chromeos::http::SendRequest(
+ data->method, data->url, data->body.c_str(), data->body.size(), mime_type,
+ {GetAuthorizationHeader()}, transport_,
+ base::Bind(&DeviceRegistrationInfo::OnCloudRequestSuccess, AsWeakPtr(),
+ data),
+ base::Bind(&DeviceRegistrationInfo::OnCloudRequestError, AsWeakPtr(),
+ data));
+ VLOG(1) << "Cloud request with ID " << request_id << " successfully sent";
+}
- std::unique_ptr<base::DictionaryValue> json_resp{
- chromeos::http::ParseJsonResponse(response.get(), nullptr, &error)};
- if (!json_resp) {
- error_callback.Run(error.get());
- return;
- }
+void DeviceRegistrationInfo::OnCloudRequestSuccess(
+ const std::shared_ptr<const CloudRequestData>& data,
+ chromeos::http::RequestID request_id,
+ std::unique_ptr<chromeos::http::Response> response) {
+ int status_code = response->GetStatusCode();
+ VLOG(1) << "Response for cloud request with ID " << request_id
+ << " received with status code " << status_code;
+ if (status_code == chromeos::http::status_code::Denied) {
+ RefreshAccessToken(
+ base::Bind(&DeviceRegistrationInfo::OnAccessTokenRefreshed, AsWeakPtr(),
+ data),
+ base::Bind(&DeviceRegistrationInfo::OnAccessTokenError, AsWeakPtr(),
+ data));
+ return;
+ }
- success_callback.Run(*json_resp);
- };
+ if (status_code >= chromeos::http::status_code::InternalServerError) {
+ // Request was valid, but server failed, retry.
+ // TODO(antonm): Reconsider status codes, maybe only some require
+ // retry.
+ // TODO(antonm): Support Retry-After header.
+ RetryCloudRequest(data);
+ return;
+ }
- auto error_cb =
- [error_callback](int request_id, const chromeos::Error* error) {
- error_callback.Run(error);
- };
+ cloud_backoff_entry_->InformOfRequest(true);
- auto transport = transport_;
- auto error_callackback_with_reauthorization = base::Bind(
- [method, url, data, mime_type, transport, request_cb, error_cb,
- status_cb](DeviceRegistrationInfo* self, int request_id,
- const chromeos::Error* error) {
- status_cb.Run(RegistrationStatus::kConnecting);
- if (error->HasError(
- chromeos::errors::http::kDomain,
- std::to_string(chromeos::http::status_code::Denied))) {
- chromeos::ErrorPtr reauthorization_error;
- // Forcibly refresh the access token.
- if (!self->RefreshAccessToken(&reauthorization_error)) {
- // TODO(antonm): Check if the device has been actually removed.
- error_cb(request_id, reauthorization_error.get());
- return;
- }
- SendRequestWithRetries(method, url, data, mime_type,
- {self->GetAuthorizationHeader()}, transport, 7,
- base::Bind(request_cb), base::Bind(error_cb));
- } else {
- error_cb(request_id, error);
- }
- },
- base::Unretained(this));
+ chromeos::ErrorPtr error;
+ auto json_resp = chromeos::http::ParseJsonResponse(response.get(), nullptr,
+ &error);
+ if (!json_resp) {
+ data->error_callback.Run(error.get());
+ return;
+ }
- SendRequestWithRetries(method, url,
- data, mime_type,
- {GetAuthorizationHeader()},
- transport,
- 7,
- base::Bind(request_cb),
- error_callackback_with_reauthorization);
+ if (!response->IsSuccessful()) {
+ ParseGCDError(json_resp.get(), &error);
+ data->error_callback.Run(error.get());
+ return;
+ }
+
+ SetRegistrationStatus(RegistrationStatus::kConnected);
+ data->success_callback.Run(*json_resp);
+}
+
+void DeviceRegistrationInfo::OnCloudRequestError(
+ const std::shared_ptr<const CloudRequestData>& data,
+ chromeos::http::RequestID request_id,
+ const chromeos::Error* error) {
+ VLOG(1) << "Cloud request with ID " << request_id << " failed";
+ RetryCloudRequest(data);
+}
+
+void DeviceRegistrationInfo::RetryCloudRequest(
+ const std::shared_ptr<const CloudRequestData>& data) {
+ SetRegistrationStatus(RegistrationStatus::kConnecting);
+ cloud_backoff_entry_->InformOfRequest(false);
+ SendCloudRequest(data);
+}
+
+void DeviceRegistrationInfo::OnAccessTokenRefreshed(
+ const std::shared_ptr<const CloudRequestData>& data) {
+ SendCloudRequest(data);
+}
+
+void DeviceRegistrationInfo::OnAccessTokenError(
+ const std::shared_ptr<const CloudRequestData>& data,
+ const chromeos::Error* error) {
+ data->error_callback.Run(error);
}
void DeviceRegistrationInfo::StartDevice(
@@ -988,10 +979,8 @@
void DeviceRegistrationInfo::OnPermanentFailure() {
LOG(ERROR) << "Failed to establish notification channel.";
notification_channel_starting_ = false;
- base::MessageLoop::current()->PostTask(
- FROM_HERE,
- base::Bind(&DeviceRegistrationInfo::RunRefreshAccessToken,
- weak_factory_.GetWeakPtr()));
+ RefreshAccessToken(base::Bind(&base::DoNothing),
+ base::Bind(&IgnoreCloudError));
}
void DeviceRegistrationInfo::OnCommandCreated(
diff --git a/buffet/device_registration_info.h b/buffet/device_registration_info.h
index 52187ef..c2f28b0 100644
--- a/buffet/device_registration_info.h
+++ b/buffet/device_registration_info.h
@@ -11,10 +11,12 @@
#include <utility>
#include <vector>
+#include <base/callback.h>
#include <base/macros.h>
#include <base/memory/weak_ptr.h>
#include <base/time/time.h>
#include <base/timer/timer.h>
+#include <chromeos/backoff_entry.h>
#include <chromeos/data_encoding.h>
#include <chromeos/errors/error.h>
#include <chromeos/http/http_transport.h>
@@ -48,6 +50,10 @@
public:
using OnRegistrationChangedCallback =
base::Callback<void(RegistrationStatus)>;
+ using CloudRequestCallback =
+ base::Callback<void(const base::DictionaryValue&)>;
+ using CloudRequestErrorCallback =
+ base::Callback<void(const chromeos::Error* error)>;
DeviceRegistrationInfo(
const std::shared_ptr<CommandManager>& command_manager,
@@ -104,10 +110,11 @@
// Checks whether we have credentials generated during registration.
bool HaveRegistrationCredentials(chromeos::ErrorPtr* error);
- // Gets the full device description JSON object, or nullptr if
- // the device is not registered or communication failure.
- std::unique_ptr<base::DictionaryValue> GetDeviceInfo(
- chromeos::ErrorPtr* error);
+ // Gets the full device description JSON object asynchronously.
+ // Passes the device info as the first argument to |callback|, or nullptr if
+ // the device is not registered or in case of communication failure.
+ void GetDeviceInfo(const CloudRequestCallback& success_callback,
+ const CloudRequestErrorCallback& error_callback);
// Registers the device.
// Returns a device ID on success.
@@ -159,20 +166,16 @@
void StartDevice(chromeos::ErrorPtr* error,
const base::TimeDelta& retry_delay);
- // Checks for the valid device registration as well as refreshes
- // the device access token, if available.
- bool CheckRegistration(chromeos::ErrorPtr* error);
-
- // If we currently have an access token and it doesn't look like it
- // has expired yet, returns true immediately. Otherwise calls
- // RefreshAccessToken().
- bool MaybeRefreshAccessToken(chromeos::ErrorPtr* error);
-
// Forcibly refreshes the access token.
- bool RefreshAccessToken(chromeos::ErrorPtr* error);
+ void RefreshAccessToken(const base::Closure& success_callback,
+ const CloudRequestErrorCallback& error_callback);
- // Calls RefreshAccessToken(nullptr). Used as a closure.
- void RunRefreshAccessToken();
+ // Success callback for RefreshAccessToken().
+ void OnRefreshAccessTokenSuccess(
+ const base::Closure& success_callback,
+ const std::shared_ptr<CloudRequestErrorCallback>& error_callback,
+ chromeos::http::RequestID id,
+ std::unique_ptr<chromeos::http::Response> response);
// Parse the OAuth response, and sets registration status to
// kInvalidCredentials if our registration is no longer valid.
@@ -183,11 +186,6 @@
// restarted anytime the access_token is refreshed.
void StartNotificationChannel();
- using CloudRequestCallback =
- base::Callback<void(const base::DictionaryValue&)>;
- using CloudRequestErrorCallback =
- base::Callback<void(const chromeos::Error* error)>;
-
// Do a HTTPS request to cloud services.
// Handles many cases like reauthorization, 5xx HTTP response codes
// and device removal. It is a recommended way to do cloud API
@@ -200,6 +198,31 @@
const CloudRequestCallback& success_callback,
const CloudRequestErrorCallback& error_callback);
+ // Helper for DoCloudRequest().
+ struct CloudRequestData {
+ std::string method;
+ std::string url;
+ std::string body;
+ CloudRequestCallback success_callback;
+ CloudRequestErrorCallback error_callback;
+ };
+ void SendCloudRequest(const std::shared_ptr<const CloudRequestData>& data);
+ void OnCloudRequestSuccess(
+ const std::shared_ptr<const CloudRequestData>& data,
+ chromeos::http::RequestID request_id,
+ std::unique_ptr<chromeos::http::Response> response);
+ void OnCloudRequestError(
+ const std::shared_ptr<const CloudRequestData>& data,
+ chromeos::http::RequestID request_id,
+ const chromeos::Error* error);
+ void RetryCloudRequest(
+ const std::shared_ptr<const CloudRequestData>& data);
+ void OnAccessTokenRefreshed(
+ const std::shared_ptr<const CloudRequestData>& data);
+ void OnAccessTokenError(
+ const std::shared_ptr<const CloudRequestData>& data,
+ const chromeos::Error* error);
+
void UpdateDeviceResource(const base::Closure& on_success,
const CloudRequestErrorCallback& on_failure);
@@ -259,6 +282,10 @@
std::unique_ptr<BuffetConfig> config_;
+ // Backoff manager for DoCloudRequest() method.
+ std::unique_ptr<chromeos::BackoffEntry::Policy> cloud_backoff_policy_;
+ std::unique_ptr<chromeos::BackoffEntry> cloud_backoff_entry_;
+
const bool notifications_enabled_;
std::unique_ptr<NotificationChannel> primary_notification_channel_;
std::unique_ptr<PullChannel> pull_channel_;
diff --git a/buffet/device_registration_info_unittest.cc b/buffet/device_registration_info_unittest.cc
index c791355..0bbaeb1 100644
--- a/buffet/device_registration_info_unittest.cc
+++ b/buffet/device_registration_info_unittest.cc
@@ -6,6 +6,8 @@
#include <base/json/json_reader.h>
#include <base/json/json_writer.h>
+#include <base/message_loop/message_loop.h>
+#include <base/run_loop.h>
#include <base/values.h>
#include <chromeos/bind_lambda.h>
#include <chromeos/http/http_request.h>
@@ -200,8 +202,28 @@
return dev_reg_->PublishCommands(commands);
}
- bool CheckRegistration(chromeos::ErrorPtr* error) const {
- return dev_reg_->CheckRegistration(error);
+ bool RefreshAccessToken(chromeos::ErrorPtr* error) const {
+ base::MessageLoopForIO message_loop;
+ base::RunLoop run_loop;
+
+ bool succeeded = false;
+ auto on_success = [&run_loop, &succeeded]() {
+ succeeded = true;
+ run_loop.Quit();
+ };
+ auto on_failure = [&run_loop, &error](const chromeos::Error* in_error) {
+ if (error)
+ *error = in_error->Clone();
+ run_loop.Quit();
+ };
+ dev_reg_->RefreshAccessToken(base::Bind(on_success),
+ base::Bind(on_failure));
+ run_loop.Run();
+ return succeeded;
+ }
+
+ void SetAccessToken() {
+ dev_reg_->access_token_ = test_data::kAccessToken;
}
RegistrationStatus GetRegistrationStatus() const {
@@ -252,8 +274,8 @@
}));
}
-TEST_F(DeviceRegistrationInfoTest, CheckRegistration) {
- EXPECT_FALSE(CheckRegistration(nullptr));
+TEST_F(DeviceRegistrationInfoTest, HaveRegistrationCredentials) {
+ EXPECT_FALSE(dev_reg_->HaveRegistrationCredentials(nullptr));
EXPECT_EQ(0, transport_->GetRequestCount());
SetDefaultDeviceRegistration(&data_);
@@ -264,8 +286,9 @@
chromeos::http::request_type::kPost,
base::Bind(OAuth2Handler));
transport_->ResetRequestCount();
- EXPECT_TRUE(CheckRegistration(nullptr));
+ EXPECT_TRUE(RefreshAccessToken(nullptr));
EXPECT_EQ(1, transport_->GetRequestCount());
+ EXPECT_TRUE(dev_reg_->HaveRegistrationCredentials(nullptr));
}
TEST_F(DeviceRegistrationInfoTest, CheckAuthenticationFailure) {
@@ -279,7 +302,7 @@
base::Bind(OAuth2HandlerFail));
transport_->ResetRequestCount();
chromeos::ErrorPtr error;
- EXPECT_FALSE(CheckRegistration(&error));
+ EXPECT_FALSE(RefreshAccessToken(&error));
EXPECT_EQ(1, transport_->GetRequestCount());
EXPECT_TRUE(error->HasError(kErrorDomainOAuth2, "unable_to_authenticate"));
EXPECT_EQ(RegistrationStatus::kConnecting, GetRegistrationStatus());
@@ -296,7 +319,7 @@
base::Bind(OAuth2HandlerDeregister));
transport_->ResetRequestCount();
chromeos::ErrorPtr error;
- EXPECT_FALSE(CheckRegistration(&error));
+ EXPECT_FALSE(RefreshAccessToken(&error));
EXPECT_EQ(1, transport_->GetRequestCount());
EXPECT_TRUE(error->HasError(kErrorDomainOAuth2, "invalid_grant"));
EXPECT_EQ(RegistrationStatus::kInvalidCredentials, GetRegistrationStatus());
@@ -306,36 +329,32 @@
SetDefaultDeviceRegistration(&data_);
storage_->Save(data_);
ReloadConfig();
+ SetAccessToken();
- transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
- chromeos::http::request_type::kPost,
- base::Bind(OAuth2Handler));
transport_->AddHandler(dev_reg_->GetDeviceURL(),
chromeos::http::request_type::kGet,
base::Bind(DeviceInfoHandler));
transport_->ResetRequestCount();
- auto device_info = dev_reg_->GetDeviceInfo(nullptr);
- EXPECT_EQ(2, transport_->GetRequestCount());
- EXPECT_NE(nullptr, device_info.get());
- base::DictionaryValue* dict = nullptr;
- EXPECT_TRUE(device_info->GetAsDictionary(&dict));
- std::string id;
- EXPECT_TRUE(dict->GetString("id", &id));
- EXPECT_EQ(test_data::kDeviceId, id);
-}
+ base::MessageLoopForIO message_loop;
+ base::RunLoop run_loop;
-TEST_F(DeviceRegistrationInfoTest, GetDeviceId) {
- SetDefaultDeviceRegistration(&data_);
- storage_->Save(data_);
- ReloadConfig();
-
- transport_->AddHandler(dev_reg_->GetOAuthURL("token"),
- chromeos::http::request_type::kPost,
- base::Bind(OAuth2Handler));
- transport_->AddHandler(dev_reg_->GetDeviceURL(),
- chromeos::http::request_type::kGet,
- base::Bind(DeviceInfoHandler));
- EXPECT_EQ(test_data::kDeviceId, config_->device_id());
+ bool succeeded = false;
+ auto on_success =
+ [&run_loop, &succeeded, this](const base::DictionaryValue& info) {
+ EXPECT_EQ(1, transport_->GetRequestCount());
+ std::string id;
+ EXPECT_TRUE(info.GetString("id", &id));
+ EXPECT_EQ(test_data::kDeviceId, id);
+ succeeded = true;
+ run_loop.Quit();
+ };
+ auto on_failure = [&run_loop](const chromeos::Error* error) {
+ run_loop.Quit();
+ FAIL() << "Should not be called";
+ };
+ dev_reg_->GetDeviceInfo(base::Bind(on_success), base::Bind(on_failure));
+ run_loop.Run();
+ EXPECT_TRUE(succeeded);
}
TEST_F(DeviceRegistrationInfoTest, RegisterDevice) {
@@ -483,6 +502,11 @@
}
TEST_F(DeviceRegistrationInfoTest, UpdateCommand) {
+ SetDefaultDeviceRegistration(&data_);
+ storage_->Save(data_);
+ ReloadConfig();
+ SetAccessToken();
+
auto json_cmds = unittests::CreateDictionaryValue(R"({
'robot': {
'_jump': {
diff --git a/buffet/manager.cc b/buffet/manager.cc
index 6921f47..bcb5cd6 100644
--- a/buffet/manager.cc
+++ b/buffet/manager.cc
@@ -126,7 +126,8 @@
privet_->OnShutdown();
}
-void Manager::CheckDeviceRegistered(DBusMethodResponse<std::string> response) {
+void Manager::CheckDeviceRegistered(
+ DBusMethodResponsePtr<std::string> response) {
LOG(INFO) << "Received call to Manager.CheckDeviceRegistered()";
chromeos::ErrorPtr error;
bool registered = device_info_->HaveRegistrationCredentials(&error);
@@ -142,23 +143,34 @@
: std::string());
}
-void Manager::GetDeviceInfo(DBusMethodResponse<std::string> response) {
+void Manager::GetDeviceInfo(DBusMethodResponsePtr<std::string> response) {
LOG(INFO) << "Received call to Manager.GetDeviceInfo()";
+ std::shared_ptr<DBusMethodResponse<std::string>> shared_response =
+ std::move(response);
- chromeos::ErrorPtr error;
- auto device_info = device_info_->GetDeviceInfo(&error);
- if (!device_info) {
- response->ReplyWithError(error.get());
- return;
- }
+ device_info_->GetDeviceInfo(
+ base::Bind(&Manager::OnGetDeviceInfoSuccess,
+ weak_ptr_factory_.GetWeakPtr(), shared_response),
+ base::Bind(&Manager::OnGetDeviceInfoError,
+ weak_ptr_factory_.GetWeakPtr(), shared_response));
+}
+void Manager::OnGetDeviceInfoSuccess(
+ const std::shared_ptr<DBusMethodResponse<std::string>>& response,
+ const base::DictionaryValue& device_info) {
std::string device_info_str;
base::JSONWriter::WriteWithOptions(
- *device_info, base::JSONWriter::OPTIONS_PRETTY_PRINT, &device_info_str);
+ device_info, base::JSONWriter::OPTIONS_PRETTY_PRINT, &device_info_str);
response->Return(device_info_str);
}
-void Manager::RegisterDevice(DBusMethodResponse<std::string> response,
+void Manager::OnGetDeviceInfoError(
+ const std::shared_ptr<DBusMethodResponse<std::string>>& response,
+ const chromeos::Error* error) {
+ response->ReplyWithError(error);
+}
+
+void Manager::RegisterDevice(DBusMethodResponsePtr<std::string> response,
const std::string& ticket_id) {
LOG(INFO) << "Received call to Manager.RegisterDevice()";
@@ -177,7 +189,7 @@
response->ReplyWithError(error.get());
}
-void Manager::UpdateState(DBusMethodResponse<> response,
+void Manager::UpdateState(DBusMethodResponsePtr<> response,
const chromeos::VariantDictionary& property_set) {
chromeos::ErrorPtr error;
if (!state_manager_->SetProperties(property_set, &error))
@@ -195,7 +207,7 @@
return true;
}
-void Manager::AddCommand(DBusMethodResponse<std::string> response,
+void Manager::AddCommand(DBusMethodResponsePtr<std::string> response,
const std::string& json_command,
const std::string& in_user_role) {
std::string error_message;
@@ -222,7 +234,7 @@
response->Return(id);
}
-void Manager::GetCommand(DBusMethodResponse<std::string> response,
+void Manager::GetCommand(DBusMethodResponsePtr<std::string> response,
const std::string& id) {
const CommandInstance* command = command_manager_->FindCommand(id);
if (!command) {
@@ -236,10 +248,9 @@
response->Return(command_str);
}
-void Manager::SetCommandVisibility(
- std::unique_ptr<chromeos::dbus_utils::DBusMethodResponse<>> response,
- const std::vector<std::string>& in_names,
- const std::string& in_visibility) {
+void Manager::SetCommandVisibility(DBusMethodResponsePtr<> response,
+ const std::vector<std::string>& in_names,
+ const std::string& in_visibility) {
CommandDefinition::Visibility visibility;
chromeos::ErrorPtr error;
if (!visibility.FromString(in_visibility, &error)) {
diff --git a/buffet/manager.h b/buffet/manager.h
index 2f8fd66..4cee9cc 100644
--- a/buffet/manager.h
+++ b/buffet/manager.h
@@ -39,9 +39,13 @@
class StateManager;
template<typename... Types>
-using DBusMethodResponse =
+using DBusMethodResponsePtr =
std::unique_ptr<chromeos::dbus_utils::DBusMethodResponse<Types...>>;
+template<typename... Types>
+using DBusMethodResponse =
+ chromeos::dbus_utils::DBusMethodResponse<Types...>;
+
// The Manager is responsible for global state of Buffet. It exposes
// interfaces which affect the entire device such as device registration and
// device state.
@@ -67,9 +71,10 @@
private:
// DBus methods:
- void CheckDeviceRegistered(DBusMethodResponse<std::string> response) override;
- void GetDeviceInfo(DBusMethodResponse<std::string> response) override;
- void RegisterDevice(DBusMethodResponse<std::string> response,
+ void CheckDeviceRegistered(
+ DBusMethodResponsePtr<std::string> response) override;
+ void GetDeviceInfo(DBusMethodResponsePtr<std::string> response) override;
+ void RegisterDevice(DBusMethodResponsePtr<std::string> response,
const std::string& ticket_id) override;
bool UpdateDeviceInfo(chromeos::ErrorPtr* error,
const std::string& in_name,
@@ -81,18 +86,17 @@
const std::string& api_key,
const std::string& oauth_url,
const std::string& service_url) override;
- void UpdateState(DBusMethodResponse<> response,
+ void UpdateState(DBusMethodResponsePtr<> response,
const chromeos::VariantDictionary& property_set) override;
bool GetState(chromeos::ErrorPtr* error, std::string* state) override;
- void AddCommand(DBusMethodResponse<std::string> response,
+ void AddCommand(DBusMethodResponsePtr<std::string> response,
const std::string& json_command,
const std::string& in_user_role) override;
- void GetCommand(DBusMethodResponse<std::string> response,
+ void GetCommand(DBusMethodResponsePtr<std::string> response,
const std::string& id) override;
- void SetCommandVisibility(
- std::unique_ptr<chromeos::dbus_utils::DBusMethodResponse<>> response,
- const std::vector<std::string>& in_names,
- const std::string& in_visibility) override;
+ void SetCommandVisibility(DBusMethodResponsePtr<> response,
+ const std::vector<std::string>& in_names,
+ const std::string& in_visibility) override;
std::string TestMethod(const std::string& message) override;
bool EnableWiFiBootstrapping(
chromeos::ErrorPtr* error,
@@ -105,6 +109,13 @@
const chromeos::VariantDictionary& in_options) override;
bool DisableGCDBootstrapping(chromeos::ErrorPtr* error) override;
+ void OnGetDeviceInfoSuccess(
+ const std::shared_ptr<DBusMethodResponse<std::string>>& response,
+ const base::DictionaryValue& device_info);
+ void OnGetDeviceInfoError(
+ const std::shared_ptr<DBusMethodResponse<std::string>>& response,
+ const chromeos::Error* error);
+
void StartPrivet(const privetd::Manager::Options& options,
chromeos::dbus_utils::AsyncEventSequencer* sequencer);