remove-http-response-apis (#1998)

* remove-http-response-apis
This commit is contained in:
Victor Vazquez 2021-03-31 16:53:38 -07:00 committed by GitHub
parent bae16450c9
commit 41be347002
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 108 additions and 191 deletions

View File

@ -4,6 +4,15 @@
### Breaking Changes
- Removed from `Azure::Core::Http::Request`:
- `SetUploadChunkSize`.
- `GetHTTPMessagePreBody`.
- `GetUploadChunkSize`.
- Removed from `Azure::Core::Http::RawResponse`:
- `SetHeader(std::string const& header)`
- `SetHeader(uint8_t const* const first, uint8_t const* const last)`.
- `GetMajorVersion`.
- `GetMinorVersion`.
- Removed from `Azure::Core::Http::Policies` namespace: `HttpPolicyOrder`, `TransportPolicy`, `RetryPolicy`, `RequestIdPolicy`, `TelemetryPolicy`, `BearerTokenAuthenticationPolicy`, `LogPolicy`.
- Renamed `Azure::Core::Http::RawResponse::GetBodyStream()` to `ExtractBodyStream()`.

View File

@ -229,11 +229,6 @@ namespace Azure { namespace Core { namespace Http {
bool m_retryModeEnabled{false};
bool m_isDownloadViaStream;
// This value can be used to override the default value that an http transport adapter uses to
// read and upload chunks of data from the payload body stream. If it is not set, the transport
// adapter will decide chunk size.
int64_t m_uploadChunkSize = 0;
// Expected to be called by a Retry policy to reset all headers set after this function was
// previously called
void StartTry();
@ -308,13 +303,6 @@ namespace Azure { namespace Core { namespace Http {
*/
void RemoveHeader(std::string const& name);
/**
* @brief Set upload chunk size.
*
* @param size Upload chunk size.
*/
void SetUploadChunkSize(int64_t size) { this->m_uploadChunkSize = size; }
// Methods used by transport layer (and logger) to send request
/**
* @brief Get HTTP method.
@ -336,16 +324,6 @@ namespace Azure { namespace Core { namespace Http {
*/
std::string GetHeadersAsString() const;
/**
* @brief Get HTTP message prior to HTTP body.
*/
std::string GetHTTPMessagePreBody() const;
/**
* @brief Get upload chunk size.
*/
int64_t GetUploadChunkSize() { return this->m_uploadChunkSize; }
/**
* @brief A value indicating whether download is happening via stream.
*/
@ -438,34 +416,6 @@ namespace Azure { namespace Core { namespace Http {
*/
void SetHeader(std::string const& name, std::string const& value);
/**
* @brief Set an HTTP header to the #RawResponse.
*
* @remark The \p header must contain valid header name characters (RFC 7230).
* @remark Header name, value and delimiter are expected to be in \p header.
*
* @param header The complete header to be added, in the form "name:value".
*
* @throw if \p header has an invalid header name or if the delimiter is missing.
*/
void SetHeader(std::string const& header);
/**
* @brief Set an HTTP header to the #RawResponse.
*
* @remark The string referenced by \p first and \p last must contain valid header name
* characters (RFC 7230).
* @remark Header name, value and delimiter are expected to be in the string referenced by \p
* first and \p last, in the form "name:value".
*
* @param first Reference to the start of an std::string.
* @param last Reference to the end of an std::string.
*
* @throw if the string referenced by \p first and \p last contains an invalid header name or if
* the delimiter is missing.
*/
void SetHeader(uint8_t const* const first, uint8_t const* const last);
/**
* @brief Set #Azure::Core::IO::BodyStream for this HTTP response.
*
@ -483,16 +433,6 @@ namespace Azure { namespace Core { namespace Http {
// adding getters for version and stream body. Clang will complain on Mac if we have unused
// fields in a class
/**
* @brief Get major number of the HTTP response protocol version.
*/
int32_t GetMajorVersion() const { return this->m_majorVersion; }
/**
* @brief Get minor number of the HTTP response protocol version.
*/
int32_t GetMinorVersion() const { return this->m_minorVersion; }
/**
* @brief Get HTTP status code of the HTTP response.
*/
@ -528,4 +468,53 @@ namespace Azure { namespace Core { namespace Http {
std::vector<uint8_t> const& GetBody() const { return this->m_body; }
};
namespace _detail {
struct RawResponseHelpers
{
/**
* @brief Insert a header into \p headers checking that \p headerName does not contain invalid
* characters.
*
* @param headers The headers map where to insert header.
* @param headerName The header name for the header to be inserted.
* @param headerValue The header value for the header to be inserted.
*
* @throw if \p headerName is invalid.
*/
static void InsertHeaderWithValidation(
CaseInsensitiveMap& headers,
std::string const& headerName,
std::string const& headerValue);
static void inline SetHeader(
Azure::Core::Http::RawResponse& response,
uint8_t const* const first,
uint8_t const* const last)
{
// get name and value from header
auto start = first;
auto end = std::find(start, last, ':');
if (end == last)
{
throw std::invalid_argument("Invalid header. No delimiter ':' found.");
}
// Always toLower() headers
auto headerName
= Azure::Core::_internal::StringExtensions::ToLower(std::string(start, end));
start = end + 1; // start value
while (start < last && (*start == ' ' || *start == '\t'))
{
++start;
}
end = std::find(start, last, '\r');
auto headerValue = std::string(start, end); // remove \r
response.SetHeader(headerName, headerValue);
}
};
} // namespace _detail
}}} // namespace Azure::Core::Http

View File

@ -18,21 +18,6 @@
namespace Azure { namespace Core {
namespace _detail {
/**
* @brief Insert a header into \p headers checking that \p headerName does not contain invalid
* characters.
*
* @param headers The headers map where to insert header.
* @param headerName The header name for the header to be inserted.
* @param headerValue The header value for the header to be inserted.
*
* @throw if \p headerName is invalid.
*/
void InsertHeaderWithValidation(
CaseInsensitiveMap& headers,
std::string const& headerName,
std::string const& headerValue);
inline std::string FormatEncodedUrlQueryParameters(
std::map<std::string, std::string> const& encodedQueryParameters)
{

View File

@ -141,6 +141,29 @@ void WinSocketSetBuffSize(curl_socket_t socket)
}
}
#endif
void static inline SetHeader(Azure::Core::Http::RawResponse& response, std::string const& header)
{
return Azure::Core::Http::_detail::RawResponseHelpers::SetHeader(
response,
reinterpret_cast<uint8_t const*>(header.data()),
reinterpret_cast<uint8_t const*>(header.data() + header.size()));
}
// Writes an HTTP request with RFC 7230 without the body (head line and headers)
// https://tools.ietf.org/html/rfc7230#section-3.1.1
static inline std::string GetHTTPMessagePreBody(Azure::Core::Http::Request const& request)
{
std::string httpRequest(HttpMethodToString(request.GetMethod()));
// HTTP version hardcoded to 1.1
auto const url = request.GetUrl().GetRelativeUrl();
httpRequest += " /" + url + " HTTP/1.1\r\n";
// headers
httpRequest += request.GetHeadersAsString();
return httpRequest;
}
} // namespace
using Azure::Core::Context;
@ -406,17 +429,13 @@ CURLcode CurlSession::UploadBody(Context const& context)
auto streamBody = this->m_request.GetBodyStream();
CURLcode sendResult = CURLE_OK;
int64_t uploadChunkSize = this->m_request.GetUploadChunkSize();
if (uploadChunkSize <= 0)
{
// use default size
uploadChunkSize = _detail::DefaultUploadChunkSize;
}
auto unique_buffer = std::make_unique<uint8_t[]>(static_cast<size_t>(uploadChunkSize));
auto unique_buffer
= std::make_unique<uint8_t[]>(static_cast<size_t>(_detail::DefaultUploadChunkSize));
while (true)
{
auto rawRequestLen = streamBody->Read(unique_buffer.get(), uploadChunkSize, context);
auto rawRequestLen
= streamBody->Read(unique_buffer.get(), _detail::DefaultUploadChunkSize, context);
if (rawRequestLen == 0)
{
break;
@ -435,7 +454,7 @@ CURLcode CurlSession::UploadBody(Context const& context)
CURLcode CurlSession::SendRawHttp(Context const& context)
{
// something like GET /path HTTP1.0 \r\nheaders\r\n
auto rawRequest = this->m_request.GetHTTPMessagePreBody();
auto rawRequest = GetHTTPMessagePreBody(this->m_request);
int64_t rawRequestLen = rawRequest.size();
CURLcode sendResult = m_connection->SendBuffer(
@ -867,7 +886,7 @@ int64_t CurlSession::ResponseBufferParser::Parse(
else if (this->state == ResponseParserState::Headers)
{
// will throw if header is invalid
this->m_response->SetHeader(this->m_internalBuffer);
SetHeader(*this->m_response, this->m_internalBuffer);
this->m_delimiterStartInPrevPosition = false;
start = index + 1; // jump \n
}
@ -904,7 +923,8 @@ int64_t CurlSession::ResponseBufferParser::Parse(
}
// will throw if header is invalid
this->m_response->SetHeader(buffer + start, buffer + index - 1);
Azure::Core::Http::_detail::RawResponseHelpers::SetHeader(
*this->m_response, buffer + start, buffer + index - 1);
this->m_delimiterStartInPrevPosition = false;
start = index + 1; // jump \n
}
@ -1051,14 +1071,14 @@ int64_t CurlSession::ResponseBufferParser::BuildHeader(
this->m_internalBuffer.append(start, indexOfEndOfStatusLine);
}
// will throw if header is invalid
m_response->SetHeader(this->m_internalBuffer);
SetHeader(*m_response, this->m_internalBuffer);
}
else
{
// Internal Buffer was not required, create response directly from buffer
std::string header(std::string(start, indexOfEndOfStatusLine));
// will throw if header is invalid
this->m_response->SetHeader(header);
SetHeader(*this->m_response, header);
}
// reuse buffer

View File

@ -12,7 +12,7 @@ using namespace Azure::Core;
using namespace Azure::Core::Http;
using namespace Azure::Core::IO::_internal;
void Azure::Core::_detail::InsertHeaderWithValidation(
void Azure::Core::Http::_detail::RawResponseHelpers::InsertHeaderWithValidation(
Azure::Core::CaseInsensitiveMap& headers,
std::string const& headerName,
std::string const& headerValue)

View File

@ -18,41 +18,9 @@ std::string const& RawResponse::GetReasonPhrase() const { return m_reasonPhrase;
Azure::Core::CaseInsensitiveMap const& RawResponse::GetHeaders() const { return this->m_headers; }
void RawResponse::SetHeader(uint8_t const* const first, uint8_t const* const last)
{
// get name and value from header
auto start = first;
auto end = std::find(start, last, ':');
if (end == last)
{
throw std::invalid_argument("Invalid header. No delimiter ':' found.");
}
// Always toLower() headers
auto headerName = Azure::Core::_internal::StringExtensions::ToLower(std::string(start, end));
start = end + 1; // start value
while (start < last && (*start == ' ' || *start == '\t'))
{
++start;
}
end = std::find(start, last, '\r');
auto headerValue = std::string(start, end); // remove \r
SetHeader(headerName, headerValue);
}
void RawResponse::SetHeader(std::string const& header)
{
return SetHeader(
reinterpret_cast<uint8_t const*>(header.data()),
reinterpret_cast<uint8_t const*>(header.data() + header.size()));
}
void RawResponse::SetHeader(std::string const& name, std::string const& value)
{
return _detail::InsertHeaderWithValidation(this->m_headers, name, value);
return _detail::RawResponseHelpers::InsertHeaderWithValidation(this->m_headers, name, value);
}
void RawResponse::SetBodyStream(std::unique_ptr<BodyStream> stream)

View File

@ -25,9 +25,10 @@ static Azure::Core::CaseInsensitiveMap MergeMaps(
void Request::SetHeader(std::string const& name, std::string const& value)
{
auto headerNameLowerCase = Azure::Core::_internal::StringExtensions::ToLower(name);
return this->m_retryModeEnabled
? _detail::InsertHeaderWithValidation(this->m_retryHeaders, headerNameLowerCase, value)
: _detail::InsertHeaderWithValidation(this->m_headers, headerNameLowerCase, value);
return this->m_retryModeEnabled ? _detail::RawResponseHelpers::InsertHeaderWithValidation(
this->m_retryHeaders, headerNameLowerCase, value)
: _detail::RawResponseHelpers::InsertHeaderWithValidation(
this->m_headers, headerNameLowerCase, value);
}
void Request::RemoveHeader(std::string const& name)
@ -73,18 +74,3 @@ std::string Request::GetHeadersAsString() const
return requestHeaderString;
}
// Writes an HTTP request with RFC 7230 without the body (head line and headers)
// https://tools.ietf.org/html/rfc7230#section-3.1.1
std::string Request::GetHTTPMessagePreBody() const
{
std::string httpRequest(HttpMethodToString(this->m_method));
// HTTP version harcoded to 1.0
auto const url = this->m_url.GetRelativeUrl();
httpRequest += " /" + url + " HTTP/1.1\r\n";
// headers
httpRequest += GetHeadersAsString();
return httpRequest;
}

View File

@ -164,8 +164,10 @@ void SetHeaders(std::string const& headers, std::unique_ptr<RawResponse>& rawRes
auto delimiter = std::find(begin, end, '\0');
if (delimiter < end)
{
rawResponse->SetHeader(
reinterpret_cast<uint8_t const*>(begin), reinterpret_cast<uint8_t const*>(delimiter));
Azure::Core::Http::_detail::RawResponseHelpers::SetHeader(
*rawResponse,
reinterpret_cast<uint8_t const*>(begin),
reinterpret_cast<uint8_t const*>(delimiter));
}
else
{
@ -269,20 +271,12 @@ void WinHttpTransport::Upload(std::unique_ptr<_detail::HandleManager>& handleMan
auto streamBody = handleManager->m_request.GetBodyStream();
int64_t streamLength = streamBody->Length();
int64_t uploadChunkSize = handleManager->m_request.GetUploadChunkSize();
if (uploadChunkSize <= 0)
// Consider using `MaximumUploadChunkSize` here, after some perf measurements
int64_t uploadChunkSize = _detail::DefaultUploadChunkSize;
if (streamLength < _detail::MaximumUploadChunkSize)
{
// use default size
if (streamLength < _detail::MaximumUploadChunkSize)
{
uploadChunkSize = streamLength;
}
else
{
uploadChunkSize = _detail::DefaultUploadChunkSize;
}
uploadChunkSize = streamLength;
}
auto unique_buffer = std::make_unique<uint8_t[]>(static_cast<size_t>(uploadChunkSize));
while (true)

View File

@ -83,15 +83,11 @@ TEST(ClientOptions, copyWithOperator)
EXPECT_EQ(1, copyOptions.PerOperationPolicies.size());
result = copyOptions.PerOperationPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(3, result->GetMajorVersion());
EXPECT_EQ(3, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerCallPolicy"), result->GetReasonPhrase());
EXPECT_EQ(1, copyOptions.PerRetryPolicies.size());
result = copyOptions.PerRetryPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(6, result->GetMajorVersion());
EXPECT_EQ(6, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerRetryPolicy"), result->GetReasonPhrase());
}
@ -118,15 +114,11 @@ TEST(ClientOptions, copyWithConstructor)
EXPECT_EQ(1, copyOptions.PerOperationPolicies.size());
result = copyOptions.PerOperationPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(3, result->GetMajorVersion());
EXPECT_EQ(3, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerCallPolicy"), result->GetReasonPhrase());
EXPECT_EQ(1, copyOptions.PerRetryPolicies.size());
result = copyOptions.PerRetryPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(6, result->GetMajorVersion());
EXPECT_EQ(6, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerRetryPolicy"), result->GetReasonPhrase());
}
@ -160,15 +152,11 @@ TEST(ClientOptions, copyDerivedClassConstructor)
EXPECT_EQ(1, copyOptions.PerOperationPolicies.size());
result = copyOptions.PerOperationPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(3, result->GetMajorVersion());
EXPECT_EQ(3, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerCallPolicy"), result->GetReasonPhrase());
EXPECT_EQ(1, copyOptions.PerRetryPolicies.size());
result = copyOptions.PerRetryPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(6, result->GetMajorVersion());
EXPECT_EQ(6, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerRetryPolicy"), result->GetReasonPhrase());
}
@ -202,15 +190,11 @@ TEST(ClientOptions, copyDerivedClassOperator)
EXPECT_EQ(1, copyOptions.PerOperationPolicies.size());
result = copyOptions.PerOperationPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(3, result->GetMajorVersion());
EXPECT_EQ(3, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerCallPolicy"), result->GetReasonPhrase());
EXPECT_EQ(1, copyOptions.PerRetryPolicies.size());
result = copyOptions.PerRetryPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(6, result->GetMajorVersion());
EXPECT_EQ(6, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerRetryPolicy"), result->GetReasonPhrase());
}
@ -244,14 +228,10 @@ TEST(ClientOptions, moveConstruct)
EXPECT_EQ(1, copyOptions.PerOperationPolicies.size());
result = copyOptions.PerOperationPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(3, result->GetMajorVersion());
EXPECT_EQ(3, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerCallPolicy"), result->GetReasonPhrase());
EXPECT_EQ(1, copyOptions.PerRetryPolicies.size());
result = copyOptions.PerRetryPolicies[0]->Send(
r, NextHttpPolicy(0, {}), Context::GetApplicationContext());
EXPECT_EQ(6, result->GetMajorVersion());
EXPECT_EQ(6, result->GetMinorVersion());
EXPECT_EQ(std::string("IamAPerRetryPolicy"), result->GetReasonPhrase());
}

View File

@ -105,12 +105,8 @@ namespace Azure { namespace Core { namespace Test {
response.GetHeaders(),
expected2);
// Response SetHeader overload method to add from string
EXPECT_THROW(response.SetHeader("inv(): header"), std::invalid_argument);
EXPECT_THROW(response.SetHeader("no delimiter header"), std::invalid_argument);
// adding header after previous error just happened on add from string
EXPECT_NO_THROW(response.SetHeader("valid3: header3"));
EXPECT_NO_THROW(response.SetHeader("valid3", "header3"));
EXPECT_PRED2(
[](Azure::Core::CaseInsensitiveMap headers, std::pair<std::string, std::string> expected) {
auto secondtHeader = headers.begin();

View File

@ -53,9 +53,8 @@ namespace Azure { namespace Core { namespace Test {
// that raw response.
auto const& response = Poll(context);
// We can use the rawResponse from the Operation here
// Major and minor version are mocked on `PollInternal`
EXPECT_EQ(response.GetMajorVersion(), 1);
EXPECT_EQ(response.GetMinorVersion(), 0);
// status code is mocked on `PollInternal`
EXPECT_EQ("OK", response.GetReasonPhrase());
}
return Response<std::string>(m_value, std::make_unique<Http::RawResponse>(*m_rawResponse));

View File

@ -24,10 +24,6 @@
namespace Azure { namespace Core { namespace Test {
namespace Datails {
constexpr int64_t FileSize = 1024 * 100;
}
TEST_P(TransportAdapter, get)
{
Azure::Core::Url host(AzureSdkHttpbinServer::Get());
@ -329,7 +325,6 @@ namespace Azure { namespace Core { namespace Test {
auto request
= Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Put, host, &bodyRequest);
// Make transport adapter to read all stream content for uploading instead of chunks
request.SetUploadChunkSize(1024 * 1024);
{
auto response = m_pipeline->Send(request, Azure::Core::Context::GetApplicationContext());
checkResponseCode(response->GetStatusCode());
@ -449,8 +444,6 @@ 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);
// Make transport adapter to read all stream content for uploading instead of chunks
request.SetUploadChunkSize(Azure::Core::Test::Datails::FileSize);
{
auto response = m_pipeline->Send(request, Azure::Core::Context::GetApplicationContext());
checkResponseCode(response->GetStatusCode());
@ -502,8 +495,6 @@ 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);
// Make transport adapter to read more than file size (5Mb)
request.SetUploadChunkSize(Azure::Core::Test::Datails::FileSize * 5);
{
auto response = m_pipeline->Send(request, Azure::Core::Context::GetApplicationContext());
checkResponseCode(response->GetStatusCode());