From 0665efe0fea59e201e3b0bac040ef7da978a5e35 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Tue, 30 Mar 2021 17:17:24 +0000 Subject: [PATCH] unique_ptr Get() => Extract() || Send() || ExtractOrCreate() (#1984) --- sdk/core/azure-core/CHANGELOG.md | 3 +++ .../azure-core/inc/azure/core/http/http.hpp | 2 +- sdk/core/azure-core/src/http/curl/curl.cpp | 15 +++++++++------ .../curl/curl_connection_pool_private.hpp | 2 +- .../src/http/curl/curl_session_private.hpp | 4 ++-- .../azure-core/src/http/transport_policy.cpp | 2 +- .../test/ut/curl_connection_pool.cpp | 19 ++++++++++++++----- .../azure-core/test/ut/curl_session_test.cpp | 8 ++++---- .../test/ut/transport_adapter_base.cpp | 4 ++-- .../common/internal/keyvault_pipeline.hpp | 2 +- .../src/delete_key_operation.cpp | 2 +- .../blobs/protocol/blob_rest_client.hpp | 2 +- .../shares/protocol/share_rest_client.hpp | 4 ++-- 13 files changed, 42 insertions(+), 27 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index ad7f74cda..841c86b20 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -2,6 +2,9 @@ ## 1.0.0-beta.8 (Unreleased) +### Breaking Changes + +- Renamed `Azure::Core::Http::RawResponse::GetBodyStream()` to `ExtractBodyStream()`. ## 1.0.0-beta.7 (2021-03-11) diff --git a/sdk/core/azure-core/inc/azure/core/http/http.hpp b/sdk/core/azure-core/inc/azure/core/http/http.hpp index 98653f8ae..2f14fe99f 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -511,7 +511,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief Get HTTP response body as #Azure::Core::IO::BodyStream. */ - std::unique_ptr GetBodyStream() + std::unique_ptr ExtractBodyStream() { // If m_bodyStream was moved before. nullptr is returned return std::move(this->m_bodyStream); diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp index 5ea5c0a70..c58518849 100644 --- a/sdk/core/azure-core/src/http/curl/curl.cpp +++ b/sdk/core/azure-core/src/http/curl/curl.cpp @@ -161,7 +161,10 @@ std::unique_ptr CurlTransport::Send(Request& request, Context const Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Creating a new session."); auto session = std::make_unique( - request, CurlConnectionPool::GetCurlConnection(request, m_options), m_options.HttpKeepAlive); + request, + CurlConnectionPool::ExtractOrCreateCurlConnection(request, m_options), + m_options.HttpKeepAlive); + CURLcode performing; // Try to send the request. If we get CURLE_UNSUPPORTED_PROTOCOL/CURLE_SEND_ERROR back, it means @@ -184,7 +187,7 @@ std::unique_ptr CurlTransport::Send(Request& request, Context const // won't be no longer valid. session = std::make_unique( request, - CurlConnectionPool::GetCurlConnection( + CurlConnectionPool::ExtractOrCreateCurlConnection( request, m_options, getConnectionOpenIntent + 1 >= _detail::RequestPoolResetAfterConnectionFailed), @@ -202,7 +205,7 @@ std::unique_ptr CurlTransport::Send(Request& request, Context const LogMsgPrefix + "Request completed. Moving response out of session and session to response."); // Move Response out of the session - auto response = session->GetResponse(); + auto response = session->ExtractResponse(); // Move the ownership of the CurlSession (bodyStream) to the response response->SetBodyStream(std::move(session)); return response; @@ -573,7 +576,7 @@ void CurlSession::ReadStatusLineAndHeadersFromRawResponse( } } - this->m_response = parser.GetResponse(); + this->m_response = parser.ExtractResponse(); this->m_innerBufferSize = static_cast(bufferSize); this->m_lastStatusCode = this->m_response->GetStatusCode(); @@ -818,7 +821,7 @@ int64_t CurlConnection::ReadFromSocket(uint8_t* buffer, int64_t bufferSize, Cont return readBytes; } -std::unique_ptr CurlSession::GetResponse() { return std::move(this->m_response); } +std::unique_ptr CurlSession::ExtractResponse() { return std::move(this->m_response); } int64_t CurlSession::ResponseBufferParser::Parse( uint8_t const* const buffer, @@ -1113,7 +1116,7 @@ inline std::string GetConnectionKey(std::string const& host, CurlTransportOption } } // namespace -std::unique_ptr CurlConnectionPool::GetCurlConnection( +std::unique_ptr CurlConnectionPool::ExtractOrCreateCurlConnection( Request& request, CurlTransportOptions const& options, bool resetPool) diff --git a/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp b/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp index c67f44716..7edb49673 100644 --- a/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_connection_pool_private.hpp @@ -71,7 +71,7 @@ namespace Azure { namespace Core { namespace Http { * * @return #Azure::Core::Http::CurlNetworkConnection to use. */ - static std::unique_ptr GetCurlConnection( + static std::unique_ptr ExtractOrCreateCurlConnection( Request& request, CurlTransportOptions const& options, bool resetPool = false); diff --git a/sdk/core/azure-core/src/http/curl/curl_session_private.hpp b/sdk/core/azure-core/src/http/curl/curl_session_private.hpp index 1e82d512e..40cb053ad 100644 --- a/sdk/core/azure-core/src/http/curl/curl_session_private.hpp +++ b/sdk/core/azure-core/src/http/curl/curl_session_private.hpp @@ -198,7 +198,7 @@ namespace Azure { namespace Core { namespace Http { * @return Will move the response only if parsing is completed and if the HTTP RawResponse * was not moved before. */ - std::unique_ptr GetResponse() + std::unique_ptr ExtractResponse() { if (m_parseCompleted && m_response != nullptr) { @@ -404,7 +404,7 @@ namespace Azure { namespace Core { namespace Http { * @return the unique ptr to the HTTP RawResponse or null if the HTTP RawResponse is not yet * created or was moved before. */ - std::unique_ptr GetResponse(); + std::unique_ptr ExtractResponse(); /** * @brief Implement #Azure::Core::IO::BodyStream length. diff --git a/sdk/core/azure-core/src/http/transport_policy.cpp b/sdk/core/azure-core/src/http/transport_policy.cpp index b590018f3..ec438fa83 100644 --- a/sdk/core/azure-core/src/http/transport_policy.cpp +++ b/sdk/core/azure-core/src/http/transport_policy.cpp @@ -57,7 +57,7 @@ std::unique_ptr TransportPolicy::Send( // If ReadToEnd fail, retry policy will eventually call this again // Using DownloadViaStream and getting an error code would also get to here to download error from // body - auto bodyStream = response->GetBodyStream(); + auto bodyStream = response->ExtractBodyStream(); response->SetBody(bodyStream->ReadToEnd(ctx)); // BodyStream is moved out of response. This makes transport implementation to clean any active // session with sockets or internal state. diff --git a/sdk/core/azure-core/test/ut/curl_connection_pool.cpp b/sdk/core/azure-core/test/ut/curl_connection_pool.cpp index e2edb17c6..414b16ccb 100644 --- a/sdk/core/azure-core/test/ut/curl_connection_pool.cpp +++ b/sdk/core/azure-core/test/ut/curl_connection_pool.cpp @@ -42,7 +42,9 @@ namespace Azure { namespace Core { namespace Test { { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; - auto connection = Azure::Core::Http::CurlConnectionPool::GetCurlConnection(req, options); + auto connection + = Azure::Core::Http::CurlConnectionPool::ExtractOrCreateCurlConnection(req, options); + EXPECT_EQ(connection->GetConnectionKey(), expectedConnectionKey); auto session = std::make_unique( @@ -62,7 +64,9 @@ namespace Azure { namespace Core { namespace Test { { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; - auto connection = Azure::Core::Http::CurlConnectionPool::GetCurlConnection(req, options); + auto connection + = Azure::Core::Http::CurlConnectionPool::ExtractOrCreateCurlConnection(req, options); + // There was just one connection in the pool, it should be empty now EXPECT_EQ(Azure::Core::Http::CurlConnectionPool::ConnectionPoolIndex.size(), 0); // And the connection key for the connection we got is the expected @@ -87,7 +91,9 @@ namespace Azure { namespace Core { namespace Test { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; options.CAInfo = CAinfo; - auto connection = Azure::Core::Http::CurlConnectionPool::GetCurlConnection(req, options); + auto connection + = Azure::Core::Http::CurlConnectionPool::ExtractOrCreateCurlConnection(req, options); + EXPECT_EQ(connection->GetConnectionKey(), secondExpectedKey); // One connection still in the pool after getting a new connection and with first expected // key @@ -120,7 +126,9 @@ namespace Azure { namespace Core { namespace Test { // Creating a new connection with default options Azure::Core::Http::CurlTransportOptions options; options.CAInfo = CAinfo; - auto connection = Azure::Core::Http::CurlConnectionPool::GetCurlConnection(req, options); + auto connection + = Azure::Core::Http::CurlConnectionPool::ExtractOrCreateCurlConnection(req, options); + EXPECT_EQ(connection->GetConnectionKey(), secondExpectedKey); // One connection still in the pool after getting a new connection and with first expected // key @@ -178,7 +186,8 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Http::HttpMethod::Get, Azure::Core::Url("http://httpbin.org/get")); Azure::Core::Http::CurlTransportOptions options; - auto connection = Azure::Core::Http::CurlConnectionPool::GetCurlConnection(req, options); + auto connection + = Azure::Core::Http::CurlConnectionPool::ExtractOrCreateCurlConnection(req, options); // Simulate connection lost (like server disconnection). connection->Shutdown(); diff --git a/sdk/core/azure-core/test/ut/curl_session_test.cpp b/sdk/core/azure-core/test/ut/curl_session_test.cpp index 7c5fdcdeb..e7cca56a4 100644 --- a/sdk/core/azure-core/test/ut/curl_session_test.cpp +++ b/sdk/core/azure-core/test/ut/curl_session_test.cpp @@ -117,9 +117,9 @@ namespace Azure { namespace Core { namespace Test { request, std::move(uniqueCurlMock), true); EXPECT_NO_THROW(session->Perform(Azure::Core::Context::GetApplicationContext())); - auto r = session->GetResponse(); + auto r = session->ExtractResponse(); r->SetBodyStream(std::move(session)); - auto bodyS = r->GetBodyStream(); + auto bodyS = r->ExtractBodyStream(); // Read the bodyStream to get all chunks EXPECT_THROW( @@ -194,9 +194,9 @@ namespace Azure { namespace Core { namespace Test { request, std::move(uniqueCurlMock), true); EXPECT_NO_THROW(session->Perform(Azure::Core::Context::GetApplicationContext())); - auto response = session->GetResponse(); + auto response = session->ExtractResponse(); response->SetBodyStream(std::move(session)); - auto bodyS = response->GetBodyStream(); + auto bodyS = response->ExtractBodyStream(); // Read the bodyStream to get all chunks EXPECT_NO_THROW(bodyS->ReadToEnd(Azure::Core::Context::GetApplicationContext())); diff --git a/sdk/core/azure-core/test/ut/transport_adapter_base.cpp b/sdk/core/azure-core/test/ut/transport_adapter_base.cpp index 280c9a029..bd2fb13af 100644 --- a/sdk/core/azure-core/test/ut/transport_adapter_base.cpp +++ b/sdk/core/azure-core/test/ut/transport_adapter_base.cpp @@ -531,7 +531,7 @@ namespace Azure { namespace Core { namespace Test { int64_t size, std::string expectedBody) { - auto body = response.GetBodyStream(); + auto body = response.ExtractBodyStream(); EXPECT_EQ(body, nullptr); std::vector bodyVector = response.GetBody(); int64_t bodySize = bodyVector.size(); @@ -553,7 +553,7 @@ namespace Azure { namespace Core { namespace Test { int64_t size, std::string expectedBody) { - auto body = response.GetBodyStream(); + auto body = response.ExtractBodyStream(); EXPECT_NE(body, nullptr); std::vector bodyVector diff --git a/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp b/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp index d6d88d497..074791901 100644 --- a/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp +++ b/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp @@ -141,7 +141,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Common { n * @param path The path for the request. * @return A unique ptr to an Http raw response. */ - std::unique_ptr GetResponse( + std::unique_ptr Send( Azure::Core::Context const& context, Azure::Core::Http::HttpMethod method, std::vector const& path) diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp index a6f39ad18..cdaf777c8 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp @@ -33,7 +33,7 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation::PollInternal(Azure::Core::C std::unique_ptr rawResponse; if (!IsDone()) { - rawResponse = m_pipeline->GetResponse( + rawResponse = m_pipeline->Send( context, Azure::Core::Http::HttpMethod::Get, {_detail::DeletedKeysPath, m_value.Name()}); m_status = CheckCompleted(*rawResponse); if (m_status == Azure::Core::OperationStatus::Succeeded) diff --git a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp index ad6e3da5d..c92f5d623 100644 --- a/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp +++ b/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/protocol/blob_rest_client.hpp @@ -4747,7 +4747,7 @@ namespace Azure { namespace Storage { namespace Blobs { { throw StorageException::CreateFromResponse(std::move(pHttpResponse)); } - response.BodyStream = httpResponse.GetBodyStream(); + response.BodyStream = httpResponse.ExtractBodyStream(); { const auto& headers = httpResponse.GetHeaders(); auto content_md5_iterator = headers.find("content-md5"); diff --git a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp index ed3c41828..f3ddb87c6 100644 --- a/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp +++ b/sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/protocol/share_rest_client.hpp @@ -5984,7 +5984,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { { // Succeeded to read the entire file. FileDownloadResult result; - result.BodyStream = response.GetBodyStream(); + result.BodyStream = response.ExtractBodyStream(); result.LastModified = DateTime::Parse( response.GetHeaders().at(_detail::HeaderLastModified), DateTime::DateFormat::Rfc1123); @@ -6141,7 +6141,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { { // Succeeded to read a specified range of the file. FileDownloadResult result; - result.BodyStream = response.GetBodyStream(); + result.BodyStream = response.ExtractBodyStream(); result.LastModified = DateTime::Parse( response.GetHeaders().at(_detail::HeaderLastModified), DateTime::DateFormat::Rfc1123);