Replace type of method parameter of HttpClient::SendRequest with enum
This way we advertise subset of of methods we are going to use.
BUG:24267885
Change-Id: I4dda539269b490d0f6a828342ee9182f539b225b
Reviewed-on: https://weave-review.googlesource.com/1294
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 5cb4395..f8172bc 100644
--- a/libweave/examples/ubuntu/curl_http_client.cc
+++ b/libweave/examples/ubuntu/curl_http_client.cc
@@ -7,6 +7,7 @@
#include <base/bind.h>
#include <curl/curl.h>
#include <weave/provider/task_runner.h>
+#include <weave/enum_to_string.h>
namespace weave {
namespace examples {
@@ -40,7 +41,7 @@
FROM_HERE, base::Bind(error_callback, base::Owned(error.release())), {});
}
-void CurlHttpClient::SendRequest(const std::string& method,
+void CurlHttpClient::SendRequest(Method method,
const std::string& url,
const Headers& headers,
const std::string& data,
@@ -50,13 +51,18 @@
&curl_easy_cleanup};
CHECK(curl);
- if (method == "GET") {
+ switch (method) {
+ case Method::kGet:
CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_HTTPGET, 1L));
- } else if (method == "POST") {
+ break;
+ case Method::kPost:
CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_HTTPPOST, 1L));
- } else {
- CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_CUSTOMREQUEST,
- method.c_str()));
+ break;
+ case Method::kPatch:
+ case Method::kPut:
+ CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_CUSTOMREQUEST,
+ weave::EnumToString(method).c_str()));
+ break;
}
CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_URL, url.c_str()));
@@ -67,7 +73,7 @@
CHECK_EQ(CURLE_OK, curl_easy_setopt(curl.get(), CURLOPT_HTTPHEADER, chunk));
- if (!data.empty() || method == "POST") {
+ if (!data.empty() || method == Method::kPost) {
CHECK_EQ(CURLE_OK,
curl_easy_setopt(curl.get(), CURLOPT_POSTFIELDS, data.c_str()));
}
diff --git a/libweave/examples/ubuntu/curl_http_client.h b/libweave/examples/ubuntu/curl_http_client.h
index 033102d..a5090ff 100644
--- a/libweave/examples/ubuntu/curl_http_client.h
+++ b/libweave/examples/ubuntu/curl_http_client.h
@@ -24,7 +24,7 @@
public:
explicit CurlHttpClient(provider::TaskRunner* task_runner);
- void SendRequest(const std::string& method,
+ void SendRequest(Method method,
const std::string& url,
const Headers& headers,
const std::string& data,
diff --git a/libweave/examples/ubuntu/event_http_client.cc b/libweave/examples/ubuntu/event_http_client.cc
index 742eef8..b24c075 100644
--- a/libweave/examples/ubuntu/event_http_client.cc
+++ b/libweave/examples/ubuntu/event_http_client.cc
@@ -93,14 +93,14 @@
EventHttpClient::EventHttpClient(EventTaskRunner* task_runner)
: task_runner_{task_runner} {}
-void EventHttpClient::SendRequest(const std::string& method,
+void EventHttpClient::SendRequest(Method method,
const std::string& url,
const Headers& headers,
const std::string& data,
const SuccessCallback& success_callback,
const ErrorCallback& error_callback) {
evhttp_cmd_type method_id;
- CHECK(weave::StringToEnum(method, &method_id));
+ CHECK(weave::StringToEnum(weave::EnumToString(method), &method_id));
std::unique_ptr<evhttp_uri, EventDeleter> http_uri{
evhttp_uri_parse(url.c_str())};
CHECK(http_uri);
@@ -146,7 +146,8 @@
return;
ErrorPtr error;
Error::AddToPrintf(&error, FROM_HERE, "http_client", "request_failed",
- "request failed: %s %s", method.c_str(), url.c_str());
+ "request failed: %s %s", EnumToString(method).c_str(),
+ url.c_str());
task_runner_->PostDelayedTask(FROM_HERE,
base::Bind(error_callback, error.get()), {});
}
diff --git a/libweave/examples/ubuntu/event_http_client.h b/libweave/examples/ubuntu/event_http_client.h
index 53a01ab..457e550 100644
--- a/libweave/examples/ubuntu/event_http_client.h
+++ b/libweave/examples/ubuntu/event_http_client.h
@@ -20,7 +20,7 @@
public:
explicit EventHttpClient(EventTaskRunner* task_runner);
- void SendRequest(const std::string& method,
+ void SendRequest(Method method,
const std::string& url,
const Headers& headers,
const std::string& data,
diff --git a/libweave/include/weave/enum_to_string.h b/libweave/include/weave/enum_to_string.h
index d6c2b79..503b753 100644
--- a/libweave/include/weave/enum_to_string.h
+++ b/libweave/include/weave/enum_to_string.h
@@ -52,7 +52,7 @@
return m.name;
}
}
- NOTREACHED();
+ NOTREACHED() << static_cast<int>(id);
return std::string();
}
diff --git a/libweave/include/weave/provider/http_client.h b/libweave/include/weave/provider/http_client.h
index 995e7d6..24b4588 100644
--- a/libweave/include/weave/provider/http_client.h
+++ b/libweave/include/weave/provider/http_client.h
@@ -17,6 +17,13 @@
class HttpClient {
public:
+ enum class Method {
+ kGet,
+ kPatch,
+ kPost,
+ kPut,
+ };
+
class Response {
public:
virtual int GetStatusCode() const = 0;
@@ -29,7 +36,7 @@
using Headers = std::vector<std::pair<std::string, std::string>>;
using SuccessCallback = base::Callback<void(const Response&)>;
- virtual void SendRequest(const std::string& method,
+ virtual void SendRequest(Method method,
const std::string& url,
const Headers& headers,
const std::string& data,
diff --git a/libweave/include/weave/provider/test/mock_http_client.h b/libweave/include/weave/provider/test/mock_http_client.h
index 8a9f955..72cb9b8 100644
--- a/libweave/include/weave/provider/test/mock_http_client.h
+++ b/libweave/include/weave/provider/test/mock_http_client.h
@@ -28,7 +28,7 @@
~MockHttpClient() override = default;
MOCK_METHOD6(SendRequest,
- void(const std::string&,
+ void(Method,
const std::string&,
const Headers&,
const std::string&,
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc
index 7960b77..5a5fb0d 100644
--- a/libweave/src/device_registration_info.cc
+++ b/libweave/src/device_registration_info.cc
@@ -113,7 +113,7 @@
class RequestSender final {
public:
- RequestSender(const std::string& method,
+ RequestSender(HttpClient::Method method,
const std::string& url,
HttpClient* transport)
: method_{method}, url_{url}, transport_{transport} {}
@@ -122,8 +122,8 @@
const ErrorCallback& error_callback) {
static int debug_id = 0;
++debug_id;
- VLOG(1) << "Sending request. id:" << debug_id << " method:" << method_
- << " url:" << url_;
+ VLOG(1) << "Sending request. id:" << debug_id
+ << " method:" << EnumToString(method_) << " url:" << url_;
VLOG(2) << "Request data: " << data_;
auto on_success = [](int debug_id,
const HttpClient::SuccessCallback& success_callback,
@@ -175,7 +175,7 @@
return headers;
}
- std::string method_;
+ HttpClient::Method method_;
std::string url_;
std::string data_;
std::string mime_type_;
@@ -380,7 +380,8 @@
std::make_shared<base::Closure>(success_callback);
auto shared_error_callback = std::make_shared<ErrorCallback>(error_callback);
- RequestSender sender{http::kPost, GetOAuthURL("token"), http_client_};
+ RequestSender sender{HttpClient::Method::kPost, GetOAuthURL("token"),
+ http_client_};
sender.SetFormData({
{"refresh_token", GetSettings().refresh_token},
{"client_id", GetSettings().client_id},
@@ -532,8 +533,8 @@
error_callback.Run(error.get());
return;
}
- DoCloudRequest(http::kGet, GetDeviceURL(), nullptr, success_callback,
- error_callback);
+ DoCloudRequest(HttpClient::Method::kGet, GetDeviceURL(), nullptr,
+ success_callback, error_callback);
}
struct DeviceRegistrationInfo::RegisterCallbacks {
@@ -574,7 +575,7 @@
auto url = GetServiceURL("registrationTickets/" + ticket_id,
{{"key", GetSettings().api_key}});
- RequestSender sender{http::kPatch, url, http_client_};
+ RequestSender sender{HttpClient::Method::kPatch, url, http_client_};
sender.SetJsonData(req_json);
sender.Send(base::Bind(&DeviceRegistrationInfo::RegisterDeviceOnTicketSent,
weak_factory_.GetWeakPtr(), ticket_id, callbacks),
@@ -599,7 +600,7 @@
std::string url =
GetServiceURL("registrationTickets/" + ticket_id + "/finalize",
{{"key", GetSettings().api_key}});
- RequestSender{http::kPost, url, http_client_}.Send(
+ RequestSender{HttpClient::Method::kPost, url, http_client_}.Send(
base::Bind(&DeviceRegistrationInfo::RegisterDeviceOnTicketFinalized,
weak_factory_.GetWeakPtr(), callbacks),
base::Bind(&DeviceRegistrationInfo::RegisterDeviceError,
@@ -634,7 +635,8 @@
UpdateDeviceInfoTimestamp(*device_draft_response);
// Now get access_token and refresh_token
- RequestSender sender2{http::kPost, GetOAuthURL("token"), http_client_};
+ RequestSender sender2{HttpClient::Method::kPost, GetOAuthURL("token"),
+ http_client_};
sender2.SetFormData(
{{"code", auth_code},
{"client_id", GetSettings().client_id},
@@ -686,7 +688,7 @@
}
void DeviceRegistrationInfo::DoCloudRequest(
- const std::string& method,
+ HttpClient::Method method,
const std::string& url,
const base::DictionaryValue* body,
const CloudRequestCallback& success_callback,
@@ -901,8 +903,8 @@
const base::DictionaryValue& command_patch,
const base::Closure& on_success,
const base::Closure& on_error) {
- DoCloudRequest(http::kPatch, GetServiceURL("commands/" + command_id),
- &command_patch,
+ DoCloudRequest(HttpClient::Method::kPatch,
+ GetServiceURL("commands/" + command_id), &command_patch,
base::Bind(&IgnoreCloudResultWithCallback, on_success),
base::Bind(&IgnoreCloudErrorWithCallback, on_error));
}
@@ -969,7 +971,7 @@
{}, {{"lastUpdateTimeMs", last_device_resource_updated_timestamp_}});
DoCloudRequest(
- http::kPut, url, device_resource.get(),
+ HttpClient::Method::kPut, url, device_resource.get(),
base::Bind(&DeviceRegistrationInfo::OnUpdateDeviceResourceSuccess,
AsWeakPtr()),
base::Bind(&DeviceRegistrationInfo::OnUpdateDeviceResourceError,
@@ -1061,7 +1063,7 @@
fetch_commands_request_sent_ = true;
fetch_commands_request_queued_ = false;
DoCloudRequest(
- http::kGet,
+ HttpClient::Method::kGet,
GetServiceURL("commands/queue", {{"deviceId", GetSettings().cloud_id}}),
nullptr, base::Bind(&DeviceRegistrationInfo::OnFetchCommandsSuccess,
AsWeakPtr(), on_success),
@@ -1105,8 +1107,9 @@
std::unique_ptr<base::DictionaryValue> cmd_copy{command_dict->DeepCopy()};
cmd_copy->SetString("state", "aborted");
// TODO(wiley) We could consider handling this error case more gracefully.
- DoCloudRequest(http::kPut, GetServiceURL("commands/" + command_id),
- cmd_copy.get(), base::Bind(&IgnoreCloudResult),
+ DoCloudRequest(HttpClient::Method::kPut,
+ GetServiceURL("commands/" + command_id), cmd_copy.get(),
+ base::Bind(&IgnoreCloudResult),
base::Bind(&IgnoreCloudError));
} else {
// Normal command, publish it to local clients.
@@ -1197,7 +1200,7 @@
device_state_update_pending_ = true;
DoCloudRequest(
- http::kPost, GetDeviceURL("patchState"), &body,
+ HttpClient::Method::kPost, GetDeviceURL("patchState"), &body,
base::Bind(&DeviceRegistrationInfo::OnPublishStateSuccess, AsWeakPtr(),
update_id),
base::Bind(&DeviceRegistrationInfo::OnPublishStateError, AsWeakPtr()));
diff --git a/libweave/src/device_registration_info.h b/libweave/src/device_registration_info.h
index 8d230a9..1577cd4 100644
--- a/libweave/src/device_registration_info.h
+++ b/libweave/src/device_registration_info.h
@@ -176,7 +176,7 @@
// and device removal. It is a recommended way to do cloud API
// requests.
// TODO(antonm): Consider moving into some other class.
- void DoCloudRequest(const std::string& method,
+ void DoCloudRequest(provider::HttpClient::Method method,
const std::string& url,
const base::DictionaryValue* body,
const CloudRequestCallback& success_callback,
@@ -184,7 +184,7 @@
// Helper for DoCloudRequest().
struct CloudRequestData {
- std::string method;
+ provider::HttpClient::Method method;
std::string url;
std::string body;
CloudRequestCallback success_callback;
diff --git a/libweave/src/device_registration_info_unittest.cc b/libweave/src/device_registration_info_unittest.cc
index 41b6d78..49c4ccb 100644
--- a/libweave/src/device_registration_info_unittest.cc
+++ b/libweave/src/device_registration_info_unittest.cc
@@ -233,9 +233,10 @@
EXPECT_FALSE(dev_reg_->HaveRegistrationCredentials());
ReloadSettings();
- EXPECT_CALL(http_client_,
- SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _, _))
+ EXPECT_CALL(
+ http_client_,
+ SendRequest(HttpClient::Method::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
const std::string& data,
const HttpClient::SuccessCallback& callback) {
@@ -261,9 +262,10 @@
ReloadSettings();
EXPECT_EQ(GcdState::kConnecting, GetGcdState());
- EXPECT_CALL(http_client_,
- SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _, _))
+ EXPECT_CALL(
+ http_client_,
+ SendRequest(HttpClient::Method::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
const std::string& data,
const HttpClient::SuccessCallback& callback) {
@@ -289,9 +291,10 @@
ReloadSettings();
EXPECT_EQ(GcdState::kConnecting, GetGcdState());
- EXPECT_CALL(http_client_,
- SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _, _))
+ EXPECT_CALL(
+ http_client_,
+ SendRequest(HttpClient::Method::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
const std::string& data,
const HttpClient::SuccessCallback& callback) {
@@ -318,7 +321,7 @@
SetAccessToken();
EXPECT_CALL(http_client_,
- SendRequest(http::kGet, dev_reg_->GetDeviceURL(),
+ SendRequest(HttpClient::Method::kGet, dev_reg_->GetDeviceURL(),
HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
_, _, _))
.WillOnce(WithArgs<3, 4>(
@@ -382,10 +385,10 @@
std::string ticket_url = dev_reg_->GetServiceURL("registrationTickets/") +
test_data::kClaimTicketId;
- EXPECT_CALL(
- http_client_,
- SendRequest(http::kPatch, ticket_url + "?key=" + test_data::kApiKey,
- HttpClient::Headers{GetJsonHeader()}, _, _, _))
+ EXPECT_CALL(http_client_,
+ SendRequest(HttpClient::Method::kPatch,
+ ticket_url + "?key=" + test_data::kApiKey,
+ HttpClient::Headers{GetJsonHeader()}, _, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
const std::string& data,
const HttpClient::SuccessCallback& callback) {
@@ -452,7 +455,7 @@
})));
EXPECT_CALL(http_client_,
- SendRequest(http::kPost,
+ SendRequest(HttpClient::Method::kPost,
ticket_url + "/finalize?key=" + test_data::kApiKey,
HttpClient::Headers{}, _, _, _))
.WillOnce(
@@ -471,9 +474,10 @@
callback.Run(*ReplyWithJson(200, json));
})));
- EXPECT_CALL(http_client_,
- SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _, _))
+ EXPECT_CALL(
+ http_client_,
+ SendRequest(HttpClient::Method::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
const std::string& data,
const HttpClient::SuccessCallback& callback) {
@@ -572,7 +576,7 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, SetProgress) {
EXPECT_CALL(http_client_,
- SendRequest(http::kPatch, command_url_,
+ SendRequest(HttpClient::Method::kPatch, command_url_,
HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
_, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
@@ -589,7 +593,7 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, Complete) {
EXPECT_CALL(http_client_,
- SendRequest(http::kPatch, command_url_,
+ SendRequest(HttpClient::Method::kPatch, command_url_,
HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
_, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
@@ -606,7 +610,7 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, Cancel) {
EXPECT_CALL(http_client_,
- SendRequest(http::kPatch, command_url_,
+ SendRequest(HttpClient::Method::kPatch, command_url_,
HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
_, _, _))
.WillOnce(WithArgs<3, 4>(Invoke([](
diff --git a/libweave/src/http_constants.cc b/libweave/src/http_constants.cc
index e80aae4..445ff75 100644
--- a/libweave/src/http_constants.cc
+++ b/libweave/src/http_constants.cc
@@ -4,14 +4,12 @@
#include "src/http_constants.h"
+#include <weave/provider/http_client.h>
+#include <weave/enum_to_string.h>
+
namespace weave {
namespace http {
-const char kGet[] = "GET";
-const char kPatch[] = "PATCH";
-const char kPost[] = "POST";
-const char kPut[] = "PUT";
-
const char kAuthorization[] = "Authorization";
const char kContentType[] = "Content-Type";
@@ -21,4 +19,21 @@
const char kWwwFormUrlEncoded[] = "application/x-www-form-urlencoded";
} // namespace http
+
+using provider::HttpClient;
+
+namespace {
+
+const weave::EnumToStringMap<HttpClient::Method>::Map kMapMethod[] = {
+ {HttpClient::Method::kGet, "GET"},
+ {HttpClient::Method::kPost, "POST"},
+ {HttpClient::Method::kPut, "PUT"},
+ {HttpClient::Method::kPatch, "PATCH"}};
+
+} // namespace
+
+template <>
+LIBWEAVE_EXPORT EnumToStringMap<HttpClient::Method>::EnumToStringMap()
+ : EnumToStringMap(kMapMethod) {}
+
} // namespace weave
diff --git a/libweave/src/http_constants.h b/libweave/src/http_constants.h
index f219a3c..973190e 100644
--- a/libweave/src/http_constants.h
+++ b/libweave/src/http_constants.h
@@ -18,11 +18,6 @@
const int kServiceUnavailable = 503;
const int kNotSupported = 501;
-extern const char kGet[];
-extern const char kPatch[];
-extern const char kPost[];
-extern const char kPut[];
-
extern const char kAuthorization[];
extern const char kContentType[];
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc
index 8eb6a54..5e15212 100644
--- a/libweave/src/weave_unittest.cc
+++ b/libweave/src/weave_unittest.cc
@@ -36,10 +36,11 @@
namespace weave {
+using provider::HttpClient;
+using provider::Network;
+using provider::test::MockHttpClientResponse;
using test::CreateDictionaryValue;
using test::ValueToString;
-using provider::test::MockHttpClientResponse;
-using provider::Network;
const char kCommandDefs[] = R"({
"base": {
@@ -136,12 +137,12 @@
protected:
void SetUp() override {}
- void ExpectRequest(const std::string& method,
+ void ExpectRequest(HttpClient::Method method,
const std::string& url,
const std::string& json_response) {
EXPECT_CALL(http_client_, SendRequest(method, url, _, _, _, _))
.WillOnce(WithArgs<4>(Invoke([json_response](
- const provider::HttpClient::SuccessCallback& callback) {
+ const HttpClient::SuccessCallback& callback) {
std::unique_ptr<provider::test::MockHttpClientResponse> response{
new StrictMock<provider::test::MockHttpClientResponse>};
EXPECT_CALL(*response, GetStatusCode())
@@ -312,7 +313,7 @@
auto response = CreateDictionaryValue(kRegistrationResponse);
response->Set("deviceDraft", draft->DeepCopy());
ExpectRequest(
- "PATCH",
+ HttpClient::Method::kPatch,
"https://www.googleapis.com/clouddevices/v1/registrationTickets/"
"TICKET_ID?key=TEST_API_KEY",
ValueToString(*response));
@@ -320,12 +321,13 @@
response = CreateDictionaryValue(kRegistrationFinalResponse);
response->Set("deviceDraft", draft->DeepCopy());
ExpectRequest(
- "POST",
+ HttpClient::Method::kPost,
"https://www.googleapis.com/clouddevices/v1/registrationTickets/"
"TICKET_ID/finalize?key=TEST_API_KEY",
ValueToString(*response));
- ExpectRequest("POST", "https://accounts.google.com/o/oauth2/token",
+ ExpectRequest(HttpClient::Method::kPost,
+ "https://accounts.google.com/o/oauth2/token",
kAuthTokenResponse);
InitDnsSdPublishing(true, "DB");
