diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 813ef27f7..f1f1d8d6a 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -9,6 +9,7 @@ ### Breaking Changes - Removed `Context::GetApplicationContext()` in favor of a new static data member `Context::ApplicationContext`. +- Renamed `Request::IsDownloadViaStream()` to `IsBufferedDownload()`. ### Bug Fixes 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 7fa7211d4..c2beabd90 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -131,7 +131,7 @@ namespace Azure { namespace Core { namespace Http { // flag to know where to insert header bool m_retryModeEnabled{false}; - bool m_isDownloadViaStream; + bool m_isBufferedDownload{true}; // Expected to be called by a Retry policy to reset all headers set after this function was // previously called @@ -144,16 +144,16 @@ namespace Azure { namespace Core { namespace Http { * @param httpMethod HTTP method. * @param url URL. * @param bodyStream #Azure::Core::IO::BodyStream. - * @param downloadViaStream A boolean value indicating whether download should happen via - * stream. + * @param bufferedDownload A boolean value indicating whether download should use a buffer + * for the response or return a body stream instead. */ explicit Request( HttpMethod httpMethod, Url url, Azure::Core::IO::BodyStream* bodyStream, - bool downloadViaStream) + bool bufferedDownload) : m_method(std::move(httpMethod)), m_url(std::move(url)), m_bodyStream(bodyStream), - m_retryModeEnabled(false), m_isDownloadViaStream(downloadViaStream) + m_retryModeEnabled(false), m_isBufferedDownload(bufferedDownload) { } @@ -165,7 +165,7 @@ namespace Azure { namespace Core { namespace Http { * @param bodyStream #Azure::Core::IO::BodyStream. */ explicit Request(HttpMethod httpMethod, Url url, Azure::Core::IO::BodyStream* bodyStream) - : Request(httpMethod, std::move(url), bodyStream, false) + : Request(httpMethod, std::move(url), bodyStream, true) { } @@ -174,10 +174,10 @@ namespace Azure { namespace Core { namespace Http { * * @param httpMethod HTTP method. * @param url URL. - * @param downloadViaStream A boolean value indicating whether download should happen via - * stream. + * @param bufferedDownload A boolean value indicating whether download should use a buffer + * for the response or return a body stream instead. */ - explicit Request(HttpMethod httpMethod, Url url, bool downloadViaStream); + explicit Request(HttpMethod httpMethod, Url url, bool bufferedDownload); /** * @brief Construct an #Azure::Core::Http::Request. @@ -224,9 +224,10 @@ namespace Azure { namespace Core { namespace Http { Azure::Core::IO::BodyStream* GetBodyStream() { return this->m_bodyStream; } /** - * @brief A value indicating whether download is happening via stream. + * @brief A value indicating whether download will return the raw response within a memory + * buffer or if it will provide a body stream instead. */ - bool IsDownloadViaStream() { return this->m_isDownloadViaStream; } + bool IsBufferedDownload() { return this->m_isBufferedDownload; } /** * @brief Get URL. diff --git a/sdk/core/azure-core/src/http/http.cpp b/sdk/core/azure-core/src/http/http.cpp index 6b9922f12..9c296cc5f 100644 --- a/sdk/core/azure-core/src/http/http.cpp +++ b/sdk/core/azure-core/src/http/http.cpp @@ -176,12 +176,12 @@ void Azure::Core::Http::_detail::RawResponseHelpers::InsertHeaderWithValidation( headers[headerName] = headerValue; } -Request::Request(HttpMethod httpMethod, Url url, bool downloadViaStream) - : Request(httpMethod, std::move(url), NullBodyStream::GetNullBodyStream(), downloadViaStream) +Request::Request(HttpMethod httpMethod, Url url, bool bufferedDownload) + : Request(httpMethod, std::move(url), NullBodyStream::GetNullBodyStream(), bufferedDownload) { } Request::Request(HttpMethod httpMethod, Url url) - : Request(httpMethod, std::move(url), NullBodyStream::GetNullBodyStream(), false) + : Request(httpMethod, std::move(url), NullBodyStream::GetNullBodyStream(), true) { } diff --git a/sdk/core/azure-core/src/http/transport_policy.cpp b/sdk/core/azure-core/src/http/transport_policy.cpp index de4c16a6f..bdb9b0ea3 100644 --- a/sdk/core/azure-core/src/http/transport_policy.cpp +++ b/sdk/core/azure-core/src/http/transport_policy.cpp @@ -40,26 +40,39 @@ std::unique_ptr TransportPolicy::Send( (void)nextHttpPolicy; ctx.ThrowIfCancelled(); - /** + /* * The transport policy is always the last policy. - * Call the transport and return + * + * Default behavior for all requests is to download the full response to the RawResponse's + * buffer. + * + ********************************** Notes ************************************************ + * + * - If ReadToEnd() fails while downloading all the response, the retry policy will make sure to + * re-send the request to re-start the download. + * + * - If the request returns error (statusCode >= 300), even if `request.IsBufferedDownload()`, the + * response will be download to the response's buffer. + * + *********************************************************************************** + * */ auto response = m_options.Transport->Send(request, ctx); auto statusCode = static_cast::type>( response->GetStatusCode()); - if (request.IsDownloadViaStream() && statusCode < 300) - { // special case to return a response with BodyStream to read directly from socket - // Return only if response is valid (less than 300) + // special case to return a response with BodyStream to read directly from socket + // Return only if response did not fail. + if (!request.IsBufferedDownload() && statusCode < 300) + { return response; } - // default behavior for all request is to download body content to Response - // 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 + // At this point, either the request is `bufferedDownload` or it return with an error code. The + // entire payload needs must be downloaded to the response's buffer. 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. return response; 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 9071e25aa..19cdde1c6 100644 --- a/sdk/core/azure-core/test/ut/transport_adapter_base.cpp +++ b/sdk/core/azure-core/test/ut/transport_adapter_base.cpp @@ -177,13 +177,13 @@ namespace Azure { namespace Core { namespace Test { { Azure::Core::Url host(AzureSdkHttpbinServer::Get()); - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, true); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length")); CheckBodyFromStream(*response, expectedResponseBodySize); - request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, true); + request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, false); // Add a header and send again. Response should return that header in the body request.SetHeader("123", "456"); response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); @@ -196,7 +196,7 @@ namespace Azure { namespace Core { namespace Test { { Azure::Core::Url host(AzureSdkHttpbinServer::Get()); - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, true); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, false); // loop sending request for (auto i = 0; i < 50; i++) @@ -213,7 +213,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Url host(AzureSdkHttpbinServer::Get()); auto expectedResponseBodySize = 0; - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Head, host, true); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Head, host, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); CheckBodyFromStream(*response, expectedResponseBodySize); @@ -231,7 +231,7 @@ namespace Azure { namespace Core { namespace Test { auto requestBodyVector = std::vector(1024, 'x'); auto bodyRequest = Azure::Core::IO::MemoryBodyStream(requestBodyVector); auto request - = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest, true); + = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); auto expectedResponseBodySize = std::stoull(response->GetHeaders().at("content-length")); @@ -247,7 +247,7 @@ namespace Azure { namespace Core { namespace Test { auto requestBodyVector = std::vector(1024, 'x'); auto bodyRequest = Azure::Core::IO::MemoryBodyStream(requestBodyVector); auto request = Azure::Core::Http::Request( - Azure::Core::Http::HttpMethod::Delete, host, &bodyRequest, true); + Azure::Core::Http::HttpMethod::Delete, host, &bodyRequest, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); @@ -263,7 +263,7 @@ namespace Azure { namespace Core { namespace Test { auto requestBodyVector = std::vector(1024, 'x'); auto bodyRequest = Azure::Core::IO::MemoryBodyStream(requestBodyVector); auto request = Azure::Core::Http::Request( - Azure::Core::Http::HttpMethod::Patch, host, &bodyRequest, true); + Azure::Core::Http::HttpMethod::Patch, host, &bodyRequest, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); @@ -282,7 +282,7 @@ namespace Azure { namespace Core { namespace Test { "response after 1 second. The server should not close the stream before all chunks are " "sent to a client."); - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, true); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); @@ -294,7 +294,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::Url host(AzureSdkHttpbinServer::Get()); std::string expectedType("This is the Response Type"); - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, false); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, host, true); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); Azure::Response responseT(expectedType, std::move(response)); @@ -341,7 +341,7 @@ namespace Azure { namespace Core { namespace Test { auto requestBodyVector = std::vector(1024, 'x'); auto bodyRequest = Azure::Core::IO::MemoryBodyStream(requestBodyVector); auto request - = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest, true); + = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest, false); auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode( response->GetStatusCode(), Azure::Core::Http::HttpStatusCode::MethodNotAllowed); @@ -506,7 +506,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::IO::FileBodyStream requestBodyStream(testDataPath); auto request = Azure::Core::Http::Request( - Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, true); + Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, false); { auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); @@ -531,7 +531,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::IO::FileBodyStream requestBodyStream(testDataPath); auto request = Azure::Core::Http::Request( - Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, true); + Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, false); // Make transport adapter to read default chunk size { auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); @@ -557,7 +557,7 @@ namespace Azure { namespace Core { namespace Test { Azure::Core::IO::FileBodyStream requestBodyStream(testDataPath); auto request = Azure::Core::Http::Request( - Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, true); + Azure::Core::Http::HttpMethod::Put, host, &requestBodyStream, false); { auto response = m_pipeline->Send(request, Azure::Core::Context::ApplicationContext); checkResponseCode(response->GetStatusCode()); 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 f3ca3ef15..1ac194be1 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 @@ -4667,7 +4667,7 @@ namespace Azure { namespace Storage { namespace Blobs { const Azure::Core::Context& context) { (void)options; - auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, url, true); + auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, url, false); request.SetHeader("x-ms-version", "2020-02-10"); if (options.Timeout.HasValue()) { 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 a64a8d7a5..bef13ae22 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 @@ -5170,7 +5170,7 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares { Azure::Core::Context context, const DownloadOptions& downloadOptions) { - Azure::Core::Http::Request request(Azure::Core::Http::HttpMethod::Get, url, true); + Azure::Core::Http::Request request(Azure::Core::Http::HttpMethod::Get, url, false); if (downloadOptions.Timeout.HasValue()) { request.GetUrl().AppendQueryParameter(