Cleanup mock_http_client.h
BUG:24267885
Change-Id: I35be9cbfb45b8b80e2db122055eab1ad40362da4
Reviewed-on: https://weave-review.googlesource.com/1291
Reviewed-by: Vitaly Buka <vitalybuka@google.com>
diff --git a/libweave/Android.mk b/libweave/Android.mk
index 36732d3..0417623 100644
--- a/libweave/Android.mk
+++ b/libweave/Android.mk
@@ -145,7 +145,6 @@
src/test/fake_stream.cc \
src/test/fake_task_runner.cc \
src/test/mock_command.cc \
- src/test/mock_http_client.cc \
src/test/unittest_utils.cc \
include $(BUILD_STATIC_LIBRARY)
diff --git a/libweave/examples/ubuntu/curl_http_client.cc b/libweave/examples/ubuntu/curl_http_client.cc
index 1d69ce5..5cb4395 100644
--- a/libweave/examples/ubuntu/curl_http_client.cc
+++ b/libweave/examples/ubuntu/curl_http_client.cc
@@ -34,12 +34,18 @@
CurlHttpClient::CurlHttpClient(provider::TaskRunner* task_runner)
: task_runner_{task_runner} {}
-std::unique_ptr<provider::HttpClient::Response>
-CurlHttpClient::SendRequestAndBlock(const std::string& method,
- const std::string& url,
- const Headers& headers,
- const std::string& data,
- ErrorPtr* error) {
+void CurlHttpClient::PostError(const ErrorCallback& error_callback,
+ ErrorPtr error) {
+ task_runner_->PostDelayedTask(
+ FROM_HERE, base::Bind(error_callback, base::Owned(error.release())), {});
+}
+
+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) {
std::unique_ptr<CURL, decltype(&curl_easy_cleanup)> curl{curl_easy_init(),
&curl_easy_cleanup};
CHECK(curl);
@@ -80,18 +86,19 @@
if (chunk)
curl_slist_free_all(chunk);
+ ErrorPtr error;
if (res != CURLE_OK) {
- Error::AddTo(error, FROM_HERE, "curl", "curl_easy_perform_error",
+ Error::AddTo(&error, FROM_HERE, "curl", "curl_easy_perform_error",
curl_easy_strerror(res));
- return nullptr;
+ return PostError(error_callback, std::move(error));
}
const std::string kContentType = "\r\nContent-Type:";
auto pos = response->content_type.find(kContentType);
if (pos == std::string::npos) {
- Error::AddTo(error, FROM_HERE, "curl", "no_content_header",
+ Error::AddTo(&error, FROM_HERE, "curl", "no_content_header",
"Content-Type header is missing");
- return nullptr;
+ return PostError(error_callback, std::move(error));
}
pos += kContentType.size();
auto pos_end = response->content_type.find("\r\n", pos);
@@ -103,30 +110,12 @@
CHECK_EQ(CURLE_OK, curl_easy_getinfo(curl.get(), CURLINFO_RESPONSE_CODE,
&response->status));
- return std::move(response);
-}
-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,
- base::Passed(&response)),
- {});
- } else {
- task_runner_->PostDelayedTask(
- FROM_HERE, base::Bind(&CurlHttpClient::RunErrorCallback,
- weak_ptr_factory_.GetWeakPtr(), error_callback,
- base::Passed(&error)),
- {});
- }
+ task_runner_->PostDelayedTask(
+ FROM_HERE, base::Bind(&CurlHttpClient::RunSuccessCallback,
+ weak_ptr_factory_.GetWeakPtr(), success_callback,
+ base::Passed(&response)),
+ {});
}
void CurlHttpClient::RunSuccessCallback(const SuccessCallback& success_callback,
@@ -134,10 +123,5 @@
success_callback.Run(*response);
}
-void CurlHttpClient::RunErrorCallback(const ErrorCallback& error_callback,
- ErrorPtr error) {
- error_callback.Run(error.get());
-}
-
} // namespace examples
} // namespace weave
diff --git a/libweave/examples/ubuntu/curl_http_client.h b/libweave/examples/ubuntu/curl_http_client.h
index 8d41c36..033102d 100644
--- a/libweave/examples/ubuntu/curl_http_client.h
+++ b/libweave/examples/ubuntu/curl_http_client.h
@@ -32,17 +32,10 @@
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 PostError(const ErrorCallback& error_callback, ErrorPtr error);
void RunSuccessCallback(const SuccessCallback& success_callback,
std::unique_ptr<Response> response);
- void RunErrorCallback(const ErrorCallback& error_callback,
- ErrorPtr error);
-
provider::TaskRunner* task_runner_{nullptr};
base::WeakPtrFactory<CurlHttpClient> weak_ptr_factory_{this};
diff --git a/libweave/include/weave/provider/test/mock_http_client.h b/libweave/include/weave/provider/test/mock_http_client.h
index ce3bfc3..8a9f955 100644
--- a/libweave/include/weave/provider/test/mock_http_client.h
+++ b/libweave/include/weave/provider/test/mock_http_client.h
@@ -27,19 +27,13 @@
public:
~MockHttpClient() override = default;
- MOCK_METHOD5(MockSendRequest,
- Response*(const std::string&,
- const std::string&,
- const Headers&,
- const std::string&,
- ErrorPtr*));
-
- 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;
+ MOCK_METHOD6(SendRequest,
+ void(const std::string&,
+ const std::string&,
+ const Headers&,
+ const std::string&,
+ const SuccessCallback&,
+ const ErrorCallback&));
};
} // namespace test
diff --git a/libweave/libweave.gypi b/libweave/libweave.gypi
index 46ad5b3..6df6c89 100644
--- a/libweave/libweave.gypi
+++ b/libweave/libweave.gypi
@@ -56,7 +56,6 @@
'src/test/fake_stream.cc',
'src/test/fake_task_runner.cc',
'src/test/mock_command.cc',
- 'src/test/mock_http_client.cc',
'src/test/unittest_utils.cc',
],
'weave_unittest_sources': [
diff --git a/libweave/libweave_standalone.gyp b/libweave/libweave_standalone.gyp
index 6c825ac..00b4251 100644
--- a/libweave/libweave_standalone.gyp
+++ b/libweave/libweave_standalone.gyp
@@ -6,9 +6,9 @@
'libraries': [
'-lcrypto',
'-lexpat',
- '-lpthread',
'-lgtest',
'-lgmock',
+ '-lpthread',
],
},
'targets': [
diff --git a/libweave/src/device_registration_info_unittest.cc b/libweave/src/device_registration_info_unittest.cc
index 4a47013..41b6d78 100644
--- a/libweave/src/device_registration_info_unittest.cc
+++ b/libweave/src/device_registration_info_unittest.cc
@@ -77,12 +77,14 @@
return {};
}
-HttpClient::Response* ReplyWithJson(int status_code, const base::Value& json) {
+std::unique_ptr<HttpClient::Response> ReplyWithJson(int status_code,
+ const base::Value& json) {
std::string text;
base::JSONWriter::WriteWithOptions(
json, base::JSONWriter::OPTIONS_PRETTY_PRINT, &text);
- MockHttpClientResponse* response = new StrictMock<MockHttpClientResponse>;
+ std::unique_ptr<MockHttpClientResponse> response{
+ new StrictMock<MockHttpClientResponse>};
EXPECT_CALL(*response, GetStatusCode())
.Times(AtLeast(1))
.WillRepeatedly(Return(status_code));
@@ -92,7 +94,7 @@
EXPECT_CALL(*response, GetData())
.Times(AtLeast(1))
.WillRepeatedly(Return(text));
- return response;
+ return std::move(response);
}
std::pair<std::string, std::string> GetAuthHeader() {
@@ -194,7 +196,6 @@
std::shared_ptr<StateManager> state_manager_;
};
-////////////////////////////////////////////////////////////////////////////////
TEST_F(DeviceRegistrationInfoTest, GetServiceURL) {
EXPECT_EQ(test_data::kServiceURL, dev_reg_->GetServiceURL());
std::string url = test_data::kServiceURL;
@@ -233,9 +234,11 @@
ReloadSettings();
EXPECT_CALL(http_client_,
- MockSendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_EQ("refresh_token", GetFormField(data, "grant_type"));
EXPECT_EQ(test_data::kRefreshToken,
GetFormField(data, "refresh_token"));
@@ -247,7 +250,7 @@
json.SetString("access_token", test_data::kAccessToken);
json.SetInteger("expires_in", 3600);
- return ReplyWithJson(200, json);
+ callback.Run(*ReplyWithJson(200, json));
})));
EXPECT_TRUE(RefreshAccessToken(nullptr));
@@ -259,9 +262,11 @@
EXPECT_EQ(GcdState::kConnecting, GetGcdState());
EXPECT_CALL(http_client_,
- MockSendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_EQ("refresh_token", GetFormField(data, "grant_type"));
EXPECT_EQ(test_data::kRefreshToken,
GetFormField(data, "refresh_token"));
@@ -271,7 +276,7 @@
base::DictionaryValue json;
json.SetString("error", "unable_to_authenticate");
- return ReplyWithJson(400, json);
+ callback.Run(*ReplyWithJson(400, json));
})));
ErrorPtr error;
@@ -285,9 +290,11 @@
EXPECT_EQ(GcdState::kConnecting, GetGcdState());
EXPECT_CALL(http_client_,
- MockSendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_EQ("refresh_token", GetFormField(data, "grant_type"));
EXPECT_EQ(test_data::kRefreshToken,
GetFormField(data, "refresh_token"));
@@ -297,7 +304,7 @@
base::DictionaryValue json;
json.SetString("error", "invalid_grant");
- return ReplyWithJson(400, json);
+ callback.Run(*ReplyWithJson(400, json));
})));
ErrorPtr error;
@@ -311,17 +318,19 @@
SetAccessToken();
EXPECT_CALL(http_client_,
- MockSendRequest(
- http::kGet, dev_reg_->GetDeviceURL(),
- HttpClient::Headers{GetAuthHeader(), GetJsonHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
- base::DictionaryValue json;
- json.SetString("channel.supportedType", "xmpp");
- json.SetString("deviceKind", "vendor");
- json.SetString("id", test_data::kDeviceId);
- json.SetString("kind", "clouddevices#device");
- return ReplyWithJson(200, json);
- })));
+ SendRequest(http::kGet, dev_reg_->GetDeviceURL(),
+ HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
+ _, _, _))
+ .WillOnce(WithArgs<3, 4>(
+ Invoke([](const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
+ base::DictionaryValue json;
+ json.SetString("channel.supportedType", "xmpp");
+ json.SetString("deviceKind", "vendor");
+ json.SetString("id", test_data::kDeviceId);
+ json.SetString("kind", "clouddevices#device");
+ callback.Run(*ReplyWithJson(200, json));
+ })));
bool succeeded = false;
auto on_success = [&succeeded, this](const base::DictionaryValue& info) {
@@ -375,9 +384,11 @@
test_data::kClaimTicketId;
EXPECT_CALL(
http_client_,
- MockSendRequest(http::kPatch, ticket_url + "?key=" + test_data::kApiKey,
- HttpClient::Headers{GetJsonHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPatch, ticket_url + "?key=" + test_data::kApiKey,
+ HttpClient::Headers{GetJsonHeader()}, _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
auto json = test::CreateDictionaryValue(data);
EXPECT_NE(nullptr, json.get());
std::string value;
@@ -437,32 +448,35 @@
device_draft->SetString("kind", "clouddevices#device");
json_resp.Set("deviceDraft", device_draft);
- return ReplyWithJson(200, json_resp);
+ callback.Run(*ReplyWithJson(200, json_resp));
})));
EXPECT_CALL(http_client_,
- MockSendRequest(http::kPost, ticket_url + "/finalize?key=" +
- test_data::kApiKey,
- HttpClient::Headers{}, _, _))
- .WillOnce(InvokeWithoutArgs([]() {
- base::DictionaryValue json;
- json.SetString("id", test_data::kClaimTicketId);
- json.SetString("kind", "clouddevices#registrationTicket");
- json.SetString("oauthClientId", test_data::kClientId);
- json.SetString("userEmail", "user@email.com");
- json.SetString("deviceDraft.id", test_data::kDeviceId);
- json.SetString("deviceDraft.kind", "clouddevices#device");
- json.SetString("deviceDraft.channel.supportedType", "xmpp");
- json.SetString("robotAccountEmail", test_data::kRobotAccountEmail);
- json.SetString("robotAccountAuthorizationCode",
- test_data::kRobotAccountAuthCode);
- return ReplyWithJson(200, json);
- }));
+ SendRequest(http::kPost,
+ ticket_url + "/finalize?key=" + test_data::kApiKey,
+ HttpClient::Headers{}, _, _, _))
+ .WillOnce(
+ WithArgs<4>(Invoke([](const HttpClient::SuccessCallback& callback) {
+ base::DictionaryValue json;
+ json.SetString("id", test_data::kClaimTicketId);
+ json.SetString("kind", "clouddevices#registrationTicket");
+ json.SetString("oauthClientId", test_data::kClientId);
+ json.SetString("userEmail", "user@email.com");
+ json.SetString("deviceDraft.id", test_data::kDeviceId);
+ json.SetString("deviceDraft.kind", "clouddevices#device");
+ json.SetString("deviceDraft.channel.supportedType", "xmpp");
+ json.SetString("robotAccountEmail", test_data::kRobotAccountEmail);
+ json.SetString("robotAccountAuthorizationCode",
+ test_data::kRobotAccountAuthCode);
+ callback.Run(*ReplyWithJson(200, json));
+ })));
EXPECT_CALL(http_client_,
- MockSendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
- HttpClient::Headers{GetFormHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPost, dev_reg_->GetOAuthURL("token"),
+ HttpClient::Headers{GetFormHeader()}, _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_EQ("authorization_code", GetFormField(data, "grant_type"));
EXPECT_EQ(test_data::kRobotAccountAuthCode, GetFormField(data, "code"));
EXPECT_EQ(test_data::kClientId, GetFormField(data, "client_id"));
@@ -479,7 +493,7 @@
json.SetString("refresh_token", test_data::kRefreshToken);
json.SetInteger("expires_in", 3600);
- return ReplyWithJson(200, json);
+ callback.Run(*ReplyWithJson(200, json));
})));
bool done = false;
@@ -558,14 +572,16 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, SetProgress) {
EXPECT_CALL(http_client_,
- MockSendRequest(
- http::kPatch, command_url_,
- HttpClient::Headers{GetAuthHeader(), GetJsonHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
- EXPECT_JSON_EQ(R"({"state":"inProgress", "progress":{"progress":18}})",
+ SendRequest(http::kPatch, command_url_,
+ HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
+ _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
+ EXPECT_JSON_EQ((R"({"state":"inProgress","progress":{"progress":18}})"),
*CreateDictionaryValue(data));
base::DictionaryValue json;
- return ReplyWithJson(200, json);
+ callback.Run(*ReplyWithJson(200, json));
})));
EXPECT_TRUE(command_->SetProgress(*CreateDictionaryValue("{'progress':18}"),
nullptr));
@@ -573,14 +589,16 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, Complete) {
EXPECT_CALL(http_client_,
- MockSendRequest(
- http::kPatch, command_url_,
- HttpClient::Headers{GetAuthHeader(), GetJsonHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPatch, command_url_,
+ HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
+ _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_JSON_EQ(R"({"state":"done", "results":{"status":"Ok"}})",
*CreateDictionaryValue(data));
base::DictionaryValue json;
- return ReplyWithJson(200, json);
+ callback.Run(*ReplyWithJson(200, json));
})));
EXPECT_TRUE(
command_->Complete(*CreateDictionaryValue("{'status': 'Ok'}"), nullptr));
@@ -588,14 +606,16 @@
TEST_F(DeviceRegistrationInfoUpdateCommandTest, Cancel) {
EXPECT_CALL(http_client_,
- MockSendRequest(
- http::kPatch, command_url_,
- HttpClient::Headers{GetAuthHeader(), GetJsonHeader()}, _, _))
- .WillOnce(WithArgs<3>(Invoke([](const std::string& data) {
+ SendRequest(http::kPatch, command_url_,
+ HttpClient::Headers{GetAuthHeader(), GetJsonHeader()},
+ _, _, _))
+ .WillOnce(WithArgs<3, 4>(Invoke([](
+ const std::string& data,
+ const HttpClient::SuccessCallback& callback) {
EXPECT_JSON_EQ(R"({"state":"cancelled"})",
*CreateDictionaryValue(data));
base::DictionaryValue json;
- return ReplyWithJson(200, json);
+ callback.Run(*ReplyWithJson(200, json));
})));
EXPECT_TRUE(command_->Cancel(nullptr));
}
diff --git a/libweave/src/test/mock_http_client.cc b/libweave/src/test/mock_http_client.cc
deleted file mode 100644
index c3ffbef..0000000
--- a/libweave/src/test/mock_http_client.cc
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright 2015 The Chromium OS Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <weave/provider/test/mock_http_client.h>
-
-#include <memory>
-#include <string>
-
-namespace weave {
-namespace provider {
-namespace test {
-
-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(*response);
- } else {
- error_callback.Run(error.get());
- }
-}
-
-} // namespace test
-} // namespace provider
-} // namespace weave
diff --git a/libweave/src/weave_unittest.cc b/libweave/src/weave_unittest.cc
index 5334e6a..8eb6a54 100644
--- a/libweave/src/weave_unittest.cc
+++ b/libweave/src/weave_unittest.cc
@@ -139,10 +139,11 @@
void ExpectRequest(const std::string& method,
const std::string& url,
const std::string& json_response) {
- EXPECT_CALL(http_client_, MockSendRequest(method, url, _, _, _))
- .WillOnce(InvokeWithoutArgs([json_response]() {
- provider::test::MockHttpClientResponse* response =
- new StrictMock<provider::test::MockHttpClientResponse>;
+ EXPECT_CALL(http_client_, SendRequest(method, url, _, _, _, _))
+ .WillOnce(WithArgs<4>(Invoke([json_response](
+ const provider::HttpClient::SuccessCallback& callback) {
+ std::unique_ptr<provider::test::MockHttpClientResponse> response{
+ new StrictMock<provider::test::MockHttpClientResponse>};
EXPECT_CALL(*response, GetStatusCode())
.Times(AtLeast(1))
.WillRepeatedly(Return(200));
@@ -152,8 +153,8 @@
EXPECT_CALL(*response, GetData())
.Times(AtLeast(1))
.WillRepeatedly(Return(json_response));
- return response;
- }));
+ callback.Run(*response);
+ })));
}
void InitConfigStore() {
