Remove request id from HTTP client
This is not used for anything but debug logging
BUG:24267885
Change-Id: Ia17a6f4288c8e2430db502bcc7efe5e392961990
Reviewed-on: https://weave-review.googlesource.com/1280
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 3e996e3..1b1e21f 100644
--- a/libweave/examples/ubuntu/curl_http_client.cc
+++ b/libweave/examples/ubuntu/curl_http_client.cc
@@ -14,9 +14,9 @@
namespace {
struct ResponseImpl : public provider::HttpClient::Response {
- int GetStatusCode() const { return status; }
- std::string GetContentType() const { return content_type; }
- const std::string& GetData() const { return data; }
+ int GetStatusCode() const override { return status; }
+ std::string GetContentType() const override { return content_type; }
+ const std::string& GetData() const override { return data; }
int status;
std::string content_type;
@@ -106,41 +106,37 @@
return std::move(response);
}
-int CurlHttpClient::SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) {
- ++request_id_;
+void CurlHttpClient::SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) {
ErrorPtr error;
auto response = SendRequestAndBlock(method, url, headers, data, &error);
if (response) {
task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&CurlHttpClient::RunSuccessCallback,
weak_ptr_factory_.GetWeakPtr(), success_callback,
- request_id_, base::Passed(&response)),
+ base::Passed(&response)),
{});
} else {
task_runner_->PostDelayedTask(
FROM_HERE, base::Bind(&CurlHttpClient::RunErrorCallback,
weak_ptr_factory_.GetWeakPtr(), error_callback,
- request_id_, base::Passed(&error)),
+ base::Passed(&error)),
{});
}
- return request_id_;
}
void CurlHttpClient::RunSuccessCallback(const SuccessCallback& success_callback,
- int id,
std::unique_ptr<Response> response) {
- success_callback.Run(id, *response);
+ success_callback.Run(*response);
}
void CurlHttpClient::RunErrorCallback(const ErrorCallback& error_callback,
- int id,
ErrorPtr error) {
- error_callback.Run(id, error.get());
+ error_callback.Run(error.get());
}
} // namespace examples
diff --git a/libweave/examples/ubuntu/curl_http_client.h b/libweave/examples/ubuntu/curl_http_client.h
index 853b797..7d62b3d 100644
--- a/libweave/examples/ubuntu/curl_http_client.h
+++ b/libweave/examples/ubuntu/curl_http_client.h
@@ -29,23 +29,20 @@
const Headers& headers,
const std::string& data,
ErrorPtr* error) override;
- int SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) override;
+ void SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) override;
private:
void RunSuccessCallback(const SuccessCallback& success_callback,
- int id,
std::unique_ptr<Response> response);
void RunErrorCallback(const ErrorCallback& error_callback,
- int id,
ErrorPtr error);
provider::TaskRunner* task_runner_{nullptr};
- int request_id_ = 0;
base::WeakPtrFactory<CurlHttpClient> weak_ptr_factory_{this};
};
diff --git a/libweave/examples/ubuntu/event_http_client.cc b/libweave/examples/ubuntu/event_http_client.cc
index bdf265c..f81916c 100644
--- a/libweave/examples/ubuntu/event_http_client.cc
+++ b/libweave/examples/ubuntu/event_http_client.cc
@@ -47,8 +47,8 @@
class EventHttpResponse : public weave::provider::HttpClient::Response {
public:
- int GetStatusCode() const { return status; }
- std::string GetContentType() const { return content_type; }
+ int GetStatusCode() const override { return status; }
+ std::string GetContentType() const override { return content_type; }
const std::string& GetData() const { return data; }
int status;
@@ -58,7 +58,6 @@
struct EventRequestState {
TaskRunner* task_runner_;
- int request_id_;
std::unique_ptr<evhttp_uri, EventDeleter> http_uri_;
std::unique_ptr<evhttp_connection, EventDeleter> evcon_;
HttpClient::SuccessCallback success_callback_;
@@ -75,9 +74,7 @@
"request failed: %s",
evutil_socket_error_to_string(err));
state->task_runner_->PostDelayedTask(
- FROM_HERE,
- base::Bind(state->error_callback_, state->request_id_, error.get()),
- {});
+ FROM_HERE, base::Bind(state->error_callback_, error.get()), {});
return;
}
std::unique_ptr<EventHttpResponse> response{new EventHttpResponse()};
@@ -88,8 +85,7 @@
auto n = evbuffer_remove(buffer, &response->data[0], length);
CHECK_EQ(n, int(length));
state->task_runner_->PostDelayedTask(
- FROM_HERE,
- base::Bind(state->success_callback_, state->request_id_, *response), {});
+ FROM_HERE, base::Bind(state->success_callback_, *response), {});
}
} // namespace
@@ -106,14 +102,12 @@
return nullptr;
}
-int EventHttpClient::SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) {
- ++request_id_;
-
+void EventHttpClient::SendRequest(const std::string& 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));
std::unique_ptr<evhttp_uri, EventDeleter> http_uri{
@@ -142,9 +136,8 @@
CHECK(conn);
std::unique_ptr<evhttp_request, EventDeleter> req{evhttp_request_new(
&RequestDoneCallback,
- new EventRequestState{task_runner_, request_id_, std::move(http_uri),
- std::move(conn), success_callback,
- error_callback})};
+ new EventRequestState{task_runner_, std::move(http_uri), std::move(conn),
+ success_callback, error_callback})};
CHECK(req);
auto output_headers = evhttp_request_get_output_headers(req.get());
evhttp_add_header(output_headers, "Host", host);
@@ -158,15 +151,13 @@
}
auto res =
evhttp_make_request(conn.get(), req.release(), method_id, uri.c_str());
- if (res < 0) {
- ErrorPtr error;
- Error::AddToPrintf(&error, FROM_HERE, "http_client", "request_failed",
- "request failed: %s %s", method.c_str(), url.c_str());
- task_runner_->PostDelayedTask(
- FROM_HERE, base::Bind(error_callback, request_id_, error.get()), {});
- return request_id_;
- }
- return request_id_;
+ if (res >= 0)
+ return;
+ ErrorPtr error;
+ Error::AddToPrintf(&error, FROM_HERE, "http_client", "request_failed",
+ "request failed: %s %s", method.c_str(), url.c_str());
+ task_runner_->PostDelayedTask(FROM_HERE,
+ base::Bind(error_callback, error.get()), {});
}
} // namespace examples
diff --git a/libweave/examples/ubuntu/event_http_client.h b/libweave/examples/ubuntu/event_http_client.h
index 65e764e..c38ad2e 100644
--- a/libweave/examples/ubuntu/event_http_client.h
+++ b/libweave/examples/ubuntu/event_http_client.h
@@ -25,16 +25,15 @@
const Headers& headers,
const std::string& data,
ErrorPtr* error) override;
- int SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) override;
+ void SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) override;
private:
EventTaskRunner* task_runner_{nullptr};
- int request_id_ = 0;
base::WeakPtrFactory<EventHttpClient> weak_ptr_factory_{this};
};
diff --git a/libweave/include/weave/provider/http_client.h b/libweave/include/weave/provider/http_client.h
index 9671f0d..cfc8937 100644
--- a/libweave/include/weave/provider/http_client.h
+++ b/libweave/include/weave/provider/http_client.h
@@ -28,8 +28,8 @@
};
using Headers = std::vector<std::pair<std::string, std::string>>;
- using SuccessCallback = base::Callback<void(int, const Response&)>;
- using ErrorCallback = base::Callback<void(int, const Error*)>;
+ 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(
@@ -39,12 +39,12 @@
const std::string& data,
ErrorPtr* error) = 0;
- virtual int SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) = 0;
+ virtual void SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) = 0;
protected:
virtual ~HttpClient() = default;
diff --git a/libweave/include/weave/provider/test/mock_http_client.h b/libweave/include/weave/provider/test/mock_http_client.h
index 21af699..86aca46 100644
--- a/libweave/include/weave/provider/test/mock_http_client.h
+++ b/libweave/include/weave/provider/test/mock_http_client.h
@@ -40,12 +40,12 @@
const std::string& data,
ErrorPtr* error) override;
- int SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) override;
+ void SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) override;
};
} // namespace test
diff --git a/libweave/src/device_registration_info.cc b/libweave/src/device_registration_info.cc
index adb1994..8975c19 100644
--- a/libweave/src/device_registration_info.cc
+++ b/libweave/src/device_registration_info.cc
@@ -124,10 +124,32 @@
data_, error);
}
- int Send(const HttpClient::SuccessCallback& success_callback,
- const HttpClient::ErrorCallback& error_callback) {
- return transport_->SendRequest(method_, url_, GetFullHeaders(), data_,
- success_callback, error_callback);
+ void Send(const HttpClient::SuccessCallback& success_callback,
+ const HttpClient::ErrorCallback& error_callback) {
+ static int debug_id = 0;
+ ++debug_id;
+ VLOG(1) << "Sending request. id:" << debug_id << " method:" << method_
+ << " url:" << url_;
+ VLOG(2) << "Request data: " << data_;
+ auto on_success = [](int debug_id,
+ const HttpClient::SuccessCallback& success_callback,
+ const HttpClient::Response& response) {
+ VLOG(1) << "Request succeeded. id:" << debug_id << " status:" <<
+ response.GetStatusCode();
+ VLOG(2) << "Response data: " << response.GetData();
+ success_callback.Run(response);
+ };
+ auto on_error = [](int debug_id,
+ const HttpClient::ErrorCallback& error_callback,
+ const Error* error) {
+ VLOG(1) << "Request failed, id=" << debug_id
+ << ", reason: " << error->GetCode()
+ << ", message: " << error->GetMessage();
+ error_callback.Run(error);
+ };
+ transport_->SendRequest(method_, url_, GetFullHeaders(), data_,
+ base::Bind(on_success, debug_id, success_callback),
+ base::Bind(on_error, debug_id, error_callback));
}
void SetAccessToken(const std::string& access_token) {
@@ -372,23 +394,20 @@
{"client_secret", GetSettings().client_secret},
{"grant_type", "refresh_token"},
});
- int request_id = sender.Send(
- base::Bind(&DeviceRegistrationInfo::OnRefreshAccessTokenSuccess,
- weak_factory_.GetWeakPtr(), shared_success_callback,
- shared_error_callback),
- base::Bind(&DeviceRegistrationInfo::OnRefreshAccessTokenError,
- weak_factory_.GetWeakPtr(), shared_success_callback,
- shared_error_callback));
- VLOG(1) << "Refresh access token request dispatched. Request ID = "
- << request_id;
+ sender.Send(base::Bind(&DeviceRegistrationInfo::OnRefreshAccessTokenSuccess,
+ weak_factory_.GetWeakPtr(), shared_success_callback,
+ shared_error_callback),
+ base::Bind(&DeviceRegistrationInfo::OnRefreshAccessTokenError,
+ weak_factory_.GetWeakPtr(), shared_success_callback,
+ shared_error_callback));
+ VLOG(1) << "Refresh access token request dispatched";
}
void DeviceRegistrationInfo::OnRefreshAccessTokenSuccess(
const std::shared_ptr<base::Closure>& success_callback,
const std::shared_ptr<ErrorCallback>& error_callback,
- int id,
const HttpClient::Response& response) {
- VLOG(1) << "Refresh access token request with ID " << id << " completed";
+ VLOG(1) << "Refresh access token request completed";
oauth2_backoff_entry_->InformOfRequest(true);
ErrorPtr error;
auto json = ParseOAuthResponse(response, &error);
@@ -424,9 +443,8 @@
void DeviceRegistrationInfo::OnRefreshAccessTokenError(
const std::shared_ptr<base::Closure>& success_callback,
const std::shared_ptr<ErrorCallback>& error_callback,
- int id,
const Error* error) {
- VLOG(1) << "Refresh access token request with ID " << id << " failed";
+ VLOG(1) << "Refresh access token failed";
oauth2_backoff_entry_->InformOfRequest(false);
RefreshAccessToken(*success_callback, *error_callback);
}
@@ -662,8 +680,6 @@
// forget about 5xx when fetching new access token).
// TODO(antonm): Add support for device removal.
- VLOG(1) << "Sending cloud request '" << data->method << "' to '" << data->url
- << "' with request body '" << data->body << "'";
ErrorPtr error;
if (!VerifyRegistrationCredentials(&error)) {
data->error_callback.Run(error.get());
@@ -684,21 +700,16 @@
RequestSender sender{data->method, data->url, http_client_};
sender.SetData(data->body, http::kJsonUtf8);
sender.SetAccessToken(access_token_);
- int request_id =
- sender.Send(base::Bind(&DeviceRegistrationInfo::OnCloudRequestSuccess,
- AsWeakPtr(), data),
- base::Bind(&DeviceRegistrationInfo::OnCloudRequestError,
- AsWeakPtr(), data));
- VLOG(1) << "Cloud request with ID " << request_id << " successfully sent";
+ sender.Send(base::Bind(&DeviceRegistrationInfo::OnCloudRequestSuccess,
+ AsWeakPtr(), data),
+ base::Bind(&DeviceRegistrationInfo::OnCloudRequestError,
+ AsWeakPtr(), data));
}
void DeviceRegistrationInfo::OnCloudRequestSuccess(
const std::shared_ptr<const CloudRequestData>& data,
- int request_id,
const HttpClient::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 == http::kDenied) {
cloud_backoff_entry_->InformOfRequest(true);
RefreshAccessToken(
@@ -746,9 +757,7 @@
void DeviceRegistrationInfo::OnCloudRequestError(
const std::shared_ptr<const CloudRequestData>& data,
- int request_id,
const Error* error) {
- VLOG(1) << "Cloud request with ID " << request_id << " failed";
RetryCloudRequest(data);
}
diff --git a/libweave/src/device_registration_info.h b/libweave/src/device_registration_info.h
index 3a57982..74bf3a8 100644
--- a/libweave/src/device_registration_info.h
+++ b/libweave/src/device_registration_info.h
@@ -155,12 +155,10 @@
void OnRefreshAccessTokenSuccess(
const std::shared_ptr<base::Closure>& success_callback,
const std::shared_ptr<ErrorCallback>& error_callback,
- int id,
const provider::HttpClient::Response& response);
void OnRefreshAccessTokenError(
const std::shared_ptr<base::Closure>& success_callback,
const std::shared_ptr<ErrorCallback>& error_callback,
- int id,
const Error* error);
// Parse the OAuth response, and sets registration status to
@@ -195,10 +193,8 @@
void SendCloudRequest(const std::shared_ptr<const CloudRequestData>& data);
void OnCloudRequestSuccess(
const std::shared_ptr<const CloudRequestData>& data,
- int request_id,
const provider::HttpClient::Response& response);
void OnCloudRequestError(const std::shared_ptr<const CloudRequestData>& data,
- int request_id,
const Error* error);
void RetryCloudRequest(const std::shared_ptr<const CloudRequestData>& data);
void OnAccessTokenRefreshed(
diff --git a/libweave/src/test/mock_http_client.cc b/libweave/src/test/mock_http_client.cc
index b17645c..94cbe08 100644
--- a/libweave/src/test/mock_http_client.cc
+++ b/libweave/src/test/mock_http_client.cc
@@ -21,21 +21,20 @@
MockSendRequest(method, url, headers, data, error)};
}
-int MockHttpClient::SendRequest(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- const SuccessCallback& success_callback,
- const ErrorCallback& error_callback) {
+void MockHttpClient::SendRequest(const std::string& method,
+ const std::string& url,
+ const Headers& headers,
+ const std::string& data,
+ const SuccessCallback& success_callback,
+ const ErrorCallback& error_callback) {
ErrorPtr error;
std::unique_ptr<Response> response{
MockSendRequest(method, url, headers, data, &error)};
if (response) {
- success_callback.Run(0, *response);
+ success_callback.Run(*response);
} else {
- error_callback.Run(0, error.get());
+ error_callback.Run(error.get());
}
- return 0;
}
} // namespace test
