github/azure-sdk-for-cpp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

make poll return const ref to rawResponse (#1875)

Browse Source 
* make poll return const ref to rawResponse
This commit is contained in:
Victor Vazquez 2021-03-11 19:18:47 -08:00 committed by GitHub
parent 6a8aff701c
commit 825d4f11d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 94 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
sdk/core/azure-core/CHANGELOG.md
View File

@ -54,6 +54,7 @@
- Renamed `azure/core/http/policy.hpp` to `azure/core/http/policies/policy.hpp`
- Renamed `azure/core/http/curl/curl.hpp` to `azure/core/http/curl_transport.hpp`
- Renamed `azure/core/http/winhttp/win_http_client.hpp` to `azure/core/http/win_http_transport.hpp`
- Changed return type from `std::unique_ptr<RawResponse>Operation::Poll()` to `RawResponse const& Operation::Poll()`.
### Bug Fixes

63
sdk/core/azure-core/inc/azure/core/operation.hpp
View File

@ -14,6 +14,7 @@
#include <chrono>
#include <memory>
#include <stdexcept>
#include <string>
namespace Azure { namespace Core {
@ -29,15 +30,52 @@ namespace Azure { namespace Core {
virtual std::unique_ptr<Http::RawResponse> PollInternal(Context& context) = 0;
virtual Response<T> PollUntilDoneInternal(std::chrono::milliseconds period, Context& context)
= 0;
virtual Azure::Core::Http::RawResponse const& GetRawResponseInternal() const = 0;
protected:
std::unique_ptr<Azure::Core::Http::RawResponse> m_rawResponse = nullptr;
OperationStatus m_status = OperationStatus::NotStarted;
Operation() = default;
// Define how an Operation<T> can be move-constructed from rvalue other. Parameter `other`
// gave up ownership for the rawResponse.
Operation(Operation&& other)
: m_rawResponse(std::move(other.m_rawResponse)), m_status(other.m_status)
{
}
// Define how an Operation<T> can be copy-constructed from some other Operation reference.
// Operation will create a clone of the rawResponse from `other`.
Operation(Operation const& other)
: m_rawResponse(std::make_unique<Http::RawResponse>(other.GetRawResponse())),
m_status(other.m_status)
{
}
// Define how an Operation<T> can be move-assigned from rvalue other. Parameter `other`
// gave up ownership for the rawResponse.
Operation& operator=(Operation&& other)
{
this->m_rawResponse = std::move(other.m_rawResponse);
this->m_status = other.m_status;
return *this;
}
// Define how an Operation<T> can be copy-assigned from some other Operation reference.
// Operation will create a clone of the rawResponse from `other`.
Operation& operator=(Operation const& other)
{
this->m_rawResponse = std::make_unique<Http::RawResponse>(other.GetRawResponse());
this->m_status = other.m_status;
return *this;
}
public:
virtual ~Operation() {}
/**
* @brief Final reuslt of the long-running operation.
* @brief Final result of the long-running operation.
*
* @return Response<T> the final result of the long-running operation.
*/
@ -56,7 +94,14 @@ namespace Azure { namespace Core {
* @return A reference to an #Azure::Core::Http::RawResponse.
* @note Does not give up ownership of the RawResponse.
*/
virtual Azure::Core::Http::RawResponse const& GetRawResponse() const = 0;
Azure::Core::Http::RawResponse const& GetRawResponse() const
{
if (!m_rawResponse)
{
throw std::runtime_error("The raw response was not yet set for the Operation.");
}
return *m_rawResponse;
};
/**
* @brief Returns the current #Azure::Core::OperationStatus of the long-running operation.
@ -88,11 +133,11 @@ namespace Azure { namespace Core {
*
* @return An HTTP #Azure::Core::Http::RawResponse returned from the service.
*/
std::unique_ptr<Http::RawResponse> Poll()
Http::RawResponse const& Poll()
{
// In the cases where the customer doesn't want to use a context we new one up and pass it
// through
return PollInternal(Context::GetApplicationContext());
return Poll(Context::GetApplicationContext());
}
/**
@ -102,7 +147,12 @@ namespace Azure { namespace Core {
*
* @return An HTTP #Azure::Core::Http::RawResponse returned from the service.
*/
std::unique_ptr<Http::RawResponse> Poll(Context& context) { return PollInternal(context); }
Http::RawResponse const& Poll(Context& context)
{
context.ThrowIfCancelled();
m_rawResponse = PollInternal(context);
return *m_rawResponse;
}
/**
* @brief Periodically calls the server till the long-running operation completes.
@ -115,7 +165,7 @@ namespace Azure { namespace Core {
{
// In the cases where the customer doesn't want to use a context we new one up and pass it
// through
return PollUntilDoneInternal(period, Context::GetApplicationContext());
return PollUntilDone(period, Context::GetApplicationContext());
}
/**
@ -128,6 +178,7 @@ namespace Azure { namespace Core {
*/
Response<T> PollUntilDone(std::chrono::milliseconds period, Context& context)
{
context.ThrowIfCancelled();
return PollUntilDoneInternal(period, context);
}
};

17
sdk/core/azure-core/test/ut/operation_test.hpp
View File

@ -21,7 +21,6 @@ namespace Azure { namespace Core { namespace Test {
private:
std::string m_operationToken;
std::string m_value;
std::unique_ptr<Azure::Core::Http::RawResponse> m_rawResponse;
private:
int m_count = 0;
@ -44,21 +43,29 @@ namespace Azure { namespace Core { namespace Test {
Response<std::string> PollUntilDoneInternal(std::chrono::milliseconds period, Context& context)
override
{
std::unique_ptr<Http::RawResponse> response;
while (!IsDone())
{
// Sleep for the period
// Actual clients should respect the retry after header if present
std::this_thread::sleep_for(period);
response = Poll(context);
// Poll gets a new rawResponse and replace it inside the Operation. Then return a ref to
// 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);
}
return Response<std::string>(m_value, std::move(response));
return Response<std::string>(m_value, std::make_unique<Http::RawResponse>(*m_rawResponse));
}
public:
Azure::Core::Http::RawResponse const& GetRawResponse() const override { return *m_rawResponse; }
Azure::Core::Http::RawResponse const& GetRawResponseInternal() const override
{
return *m_rawResponse;
}
std::string GetResumeToken() const override { return m_operationToken; }

9
sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/delete_key_operation.hpp
View File

@ -38,7 +38,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
std::shared_ptr<Azure::Security::KeyVault::Common::_internal::KeyVaultPipeline> m_pipeline;
Azure::Security::KeyVault::Keys::DeletedKey m_value;
std::unique_ptr<Azure::Core::Http::RawResponse> m_rawResponse;
std::string m_continuationToken;
/* This is the implementation for checking the status of a deleted key. The key is considered
@ -53,7 +52,8 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
{
while (true)
{
m_rawResponse = Poll(context);
// Poll will update the raw response.
Poll(context);
if (IsDone())
{
break;
@ -82,7 +82,10 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
* @return A reference to an #Azure::Core::Http::RawResponse.
* @note Does not give up ownership of the RawResponse.
*/
Azure::Core::Http::RawResponse const& GetRawResponse() const override { return *m_rawResponse; }
Azure::Core::Http::RawResponse const& GetRawResponseInternal() const override
{
return *m_rawResponse;
}
/**
* @brief Get the #Azure::Security::KeyVault::Keys::DeletedKey object.

5
sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp
View File

@ -30,9 +30,10 @@ inline Azure::Core::OperationStatus CheckCompleted(Azure::Core::Http::RawRespons
std::unique_ptr<Azure::Core::Http::RawResponse>
Azure::Security::KeyVault::Keys::DeleteKeyOperation::PollInternal(Azure::Core::Context& context)
{
std::unique_ptr<Azure::Core::Http::RawResponse> rawResponse;
if (!IsDone())
{
m_rawResponse = m_pipeline->GetResponse(
rawResponse = m_pipeline->GetResponse(
context, Azure::Core::Http::HttpMethod::Get, {_detail::DeletedKeysPath, m_value.Name()});
m_status = CheckCompleted(*m_rawResponse);
}
@ -40,7 +41,7 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation::PollInternal(Azure::Core::C
// To ensure the success of calling Poll multiple times, even after operation is completed, a
// copy of the raw http response is returned instead of transfering the ownership of the raw
// response inside the Operation.
return std::make_unique<Azure::Core::Http::RawResponse>(*m_rawResponse);
return rawResponse;
}
Azure::Security::KeyVault::Keys::DeleteKeyOperation::DeleteKeyOperation(

2
sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_delete_test_live.cpp
View File

@ -89,7 +89,7 @@ TEST_F(KeyVaultClientTest, DeleteKeyOperationPoll)
// Expected not completed operation
EXPECT_EQ(
static_cast<typename std::underlying_type<Azure::Core::Http::HttpStatusCode>::type>(
pollResponse->GetStatusCode()),
pollResponse.GetStatusCode()),
404);
}
}

5
sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_responses.hpp
View File

@ -87,7 +87,10 @@ namespace Azure { namespace Storage { namespace Blobs {
* @return A reference to an #Azure::Core::Http::RawResponse.
* @note Does not give up ownership of the RawResponse.
*/
Azure::Core::Http::RawResponse const& GetRawResponse() const override { return *m_rawResponse; }
Azure::Core::Http::RawResponse const& GetRawResponseInternal() const override
{
return *m_rawResponse;
}
StartCopyBlobOperation() = default;

7
sdk/storage/azure-storage-blobs/src/blob_responses.cpp
View File

@ -8,9 +8,8 @@
namespace Azure { namespace Storage { namespace Blobs {
std::unique_ptr<Azure::Core::Http::RawResponse> StartCopyBlobOperation::PollInternal(
Azure::Core::Context& context)
Azure::Core::Context&)
{
(void)context;
auto response = m_blobClient->GetProperties();
if (!response->CopyStatus.HasValue())
@ -39,12 +38,12 @@ namespace Azure { namespace Storage { namespace Blobs {
{
while (true)
{
auto rawResponse = PollInternal(context);
auto rawResponse = Poll(context);
if (m_status == Azure::Core::OperationStatus::Succeeded)
{
return Azure::Response<Models::GetBlobPropertiesResult>(
m_pollResult, std::move(rawResponse));
m_pollResult, std::make_unique<Azure::Core::Http::RawResponse>(rawResponse));
}
else if (m_status == Azure::Core::OperationStatus::Failed)
{

6
sdk/storage/azure-storage-files-shares/inc/azure/storage/files/shares/share_responses.hpp
View File

@ -203,7 +203,10 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
* @return A reference to an #Azure::Core::Http::RawResponse.
* @note Does not give up ownership of the RawResponse.
*/
Azure::Core::Http::RawResponse const& GetRawResponse() const override { return *m_rawResponse; }
Azure::Core::Http::RawResponse const& GetRawResponseInternal() const override
{
return *m_rawResponse;
}
StartCopyShareFileOperation() = default;
@ -227,7 +230,6 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
std::chrono::milliseconds period,
Azure::Core::Context& context) override;
std::unique_ptr<Azure::Core::Http::RawResponse> m_rawResponse;
std::shared_ptr<ShareFileClient> m_fileClient;
Models::GetShareFilePropertiesResult m_pollResult;

7
sdk/storage/azure-storage-files-shares/src/share_responses.cpp
View File

@ -10,9 +10,8 @@
namespace Azure { namespace Storage { namespace Files { namespace Shares {
std::unique_ptr<Azure::Core::Http::RawResponse> StartCopyShareFileOperation::PollInternal(
Azure::Core::Context& context)
Azure::Core::Context&)
{
(void)context;
auto response = m_fileClient->GetProperties();
if (!response->CopyStatus.HasValue())
@ -40,12 +39,12 @@ namespace Azure { namespace Storage { namespace Files { namespace Shares {
{
while (true)
{
auto rawResponse = PollInternal(context);
auto rawResponse = Poll(context);
if (m_status == Azure::Core::OperationStatus::Succeeded)
{
return Azure::Response<Models::GetShareFilePropertiesResult>(
m_pollResult, std::move(rawResponse));
m_pollResult, std::make_unique<Azure::Core::Http::RawResponse>(rawResponse));
}
else if (m_status == Azure::Core::OperationStatus::Failed)
{