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() {