LRO operation resume token - following factory method design (#2195)

* Add tests for resume token pattern

* update deleteKeyOperation

sdk/core/azure-core/test/ut/operation.cpp

@@ -88,3 +88,22 @@ TEST(Operation, Status)
   EXPECT_THROW(operation.Value(), std::runtime_error);
   EXPECT_EQ(operation.Status(), OperationStatus::Cancelled);
 }
+
+TEST(Operation, ResumeToken)
+{
+  StringClient client;
+  std::string token;
+  {
+    auto operation = client.StartStringUpdate();
+    token = operation.GetResumeToken();
+  }
+  {
+    for (auto resumedOperation = StringOperation::CreateFromResumeToken(token, client);
+         !resumedOperation.IsDone();
+         resumedOperation.Poll())
+    {
+      EXPECT_FALSE(resumedOperation.HasValue());
+      EXPECT_THROW(resumedOperation.Value(), std::runtime_error);
+    }
+  }
+}

sdk/core/azure-core/test/ut/operation_test.hpp

@@ -21,8 +21,6 @@ namespace Azure { namespace Core { namespace Test {
   private:
     std::string m_operationToken;
     std::string m_value;
-
-  private:
     int m_count = 0;
 
   private:
@@ -60,7 +58,14 @@ namespace Azure { namespace Core { namespace Test {
       return Response<std::string>(m_value, std::make_unique<Http::RawResponse>(*m_rawResponse));
     }
 
+    StringOperation(std::string const& resumeToken, StringClient const&)
+        : m_operationToken(resumeToken)
+    {
+    }
+
   public:
+    StringOperation() = default;
+
     Azure::Core::Http::RawResponse const& GetRawResponseInternal() const override
     {
       return *m_rawResponse;
@@ -81,6 +86,15 @@ namespace Azure { namespace Core { namespace Test {
     // This is a helper method to allow testing of the underlying operation<T> behaviors
     //  ClientOperations would not expose a way to control status
     void SetOperationStatus(OperationStatus status) { m_status = status; }
+
+    static StringOperation CreateFromResumeToken(
+        std::string const& resumeToken,
+        StringClient const& client)
+    {
+      StringOperation operation(resumeToken, client);
+      operation.Poll();
+      return operation;
+    }
   };
 
   class StringClient {

sdk/keyvault/azure-security-keyvault-keys/CHANGELOG.md

@@ -5,6 +5,7 @@
 ### New Features
 
 - Added support for importing and deserializing EC and OCT keys.
+- Added `CreateFromResumeToken()` to `DeletedKeyOperation` and `RecoverKeyOperation`.
 
 ### Breaking Changes
 
@@ -23,6 +24,7 @@
 - Changed the returned type for list keys, key versions, and deleted keys from `Response<T>` to `PagedResponse<T>` affecting:
   - `GetPropertiesOfKeysSinglePage()` and `GetPropertiesOfKeyVersionsSinglePage()` now returns `KeyProperties`.
   - `GetDeletedKeysSinglePage()` now returns `DeletedKey`.
+- Removed `ResumeDeleteKeyOperation()` and `ResumeRecoverKeyOperation()`.
 
 ### Bug Fixes
 

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/delete_key_operation.hpp

@@ -9,13 +9,12 @@
 
 #pragma once
 
+#include <azure/core/context.hpp>
 #include <azure/core/http/http.hpp>
 #include <azure/core/operation.hpp>
 #include <azure/core/operation_status.hpp>
 #include <azure/core/response.hpp>
 
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
-
 #include "azure/keyvault/keys/deleted_key.hpp"
 
 #include <memory>
@@ -24,6 +23,8 @@
 
 namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
 
+  class KeyClient;
+
   /**
    * @brief A long running operation to delete a key.
    *
@@ -35,7 +36,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * constructor is private and requires internal components.*/
     friend class KeyClient;
 
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> m_pipeline;
+    std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> m_keyClient;
     Azure::Security::KeyVault::Keys::DeletedKey m_value;
     std::string m_continuationToken;
 
@@ -71,13 +72,13 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * Since C++ doesn't offer `internal` access, we use friends-only instead.
      */
     DeleteKeyOperation(
-        std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
+        std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient,
         Azure::Response<Azure::Security::KeyVault::Keys::DeletedKey> response);
 
     DeleteKeyOperation(
-        std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
-        std::string resumeToken)
-        : m_pipeline(keyvaultPipeline), m_value(DeletedKey(resumeToken)),
+        std::string resumeToken,
+        std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient)
+        : m_keyClient(keyClient), m_value(DeletedKey(resumeToken)),
           m_continuationToken(std::move(resumeToken))
     {
     }
@@ -108,6 +109,30 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * @return std::string
      */
     std::string GetResumeToken() const override { return m_continuationToken; }
+
+    /**
+     * @brief Create a #DeleteKeyOperation from the \p resumeToken fetched from another
+     * `Operation<T>`, updated to the the latest operation status.
+     *
+     * @remark After the operation is initialized, it is used to poll the last update from the
+     * server using the \p context.
+     *
+     * @param resumeToken A previously generated token used to resume the polling of the operation.
+     * @param client A #KeyClient that is used for getting status updates.
+     * @param context A #Azure::Core::Context controlling the request lifetime.
+     * @return DeleteKeyOperation
+     */
+    static DeleteKeyOperation CreateFromResumeToken(
+        std::string const& resumeToken,
+        Azure::Security::KeyVault::Keys::KeyClient const& client,
+        Azure::Core::Context const& context = Azure::Core::Context())
+    {
+
+      DeleteKeyOperation operation(
+          resumeToken, std::make_shared<Azure::Security::KeyVault::Keys::KeyClient>(client));
+      operation.Poll(context);
+      return operation;
+    }
   };
 
 }}}} // namespace Azure::Security::KeyVault::Keys

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client.hpp

@@ -40,10 +40,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
    */
   struct GetKeyOptions
   {
-    /**
-     * @brief Context for cancelling long running operations.
-     */
-    Azure::Core::Context Context;
     /**
      * @brief Specify the key version to get.
      */
@@ -228,18 +224,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
         std::string const& name,
         Azure::Core::Context const& context = Azure::Core::Context()) const;
 
-    /**
-     * @brief Construct a #DeletedKeyOperation from a resume token previously generated by calling
-     * #DeleteKeyOperation::GetResumeToken after a call to #StartDeleteKey().
-     *
-     * @param resumeToken A generated token from a DeletedKeyOperation.
-     * @param context A cancellation token controlling the request lifetime.
-     * @return Azure::Security::KeyVault::Keys::DeleteKeyOperation
-     */
-    Azure::Security::KeyVault::Keys::DeleteKeyOperation ResumeDeleteKey(
-        std::string const& resumeToken,
-        Azure::Core::Context const& context = Azure::Core::Context()) const;
-
     /**
      * @brief Gets the public part of a deleted key.
      *
@@ -308,19 +292,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
         std::string const& name,
         Azure::Core::Context const& context = Azure::Core::Context()) const;
 
-    /**
-     * @brief Construct a #RecoverDeletedKeyOperation from a resume token previously generated by
-     * calling #RecoverDeletedKeyOperation::GetResumeToken after a call to
-     * #StartRecoverDeletedKey().
-     *
-     * @param resumeToken A generated token from a StartRecoverDeletedKey.
-     * @param context A cancellation token controlling the request lifetime.
-     * @return Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation
-     */
-    Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation ResumeRecoverDeletedKey(
-        std::string const& resumeToken,
-        Azure::Core::Context const& context = Azure::Core::Context()) const;
-
     /**
      * @brief  The update key operation changes specified attributes of a stored key and can be
      * applied to any key type and key version stored in Azure Key Vault.

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/recover_deleted_key_operation.hpp

@@ -15,8 +15,6 @@
 #include <azure/core/operation_status.hpp>
 #include <azure/core/response.hpp>
 
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
-
 #include "azure/keyvault/keys/key_vault_key.hpp"
 
 #include <memory>
@@ -25,6 +23,8 @@
 
 namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
 
+  class KeyClient;
+
   /**
    * @brief A long running operation to recover a key.
    *
@@ -35,7 +35,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * The constructor is private and requires internal components.*/
     friend class KeyClient;
 
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> m_pipeline;
+    std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> m_keyClient;
     Azure::Security::KeyVault::Keys::KeyVaultKey m_value;
     std::string m_continuationToken;
 
@@ -68,13 +68,13 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * Since C++ doesn't offer `internal` access, we use friends-only instead.
      */
     RecoverDeletedKeyOperation(
-        std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
+        std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient,
         Azure::Response<Azure::Security::KeyVault::Keys::KeyVaultKey> response);
 
     RecoverDeletedKeyOperation(
-        std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
-        std::string resumeToken)
-        : m_pipeline(keyvaultPipeline), m_value(DeletedKey(resumeToken)),
+        std::string resumeToken,
+        std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient)
+        : m_keyClient(keyClient), m_value(DeletedKey(resumeToken)),
           m_continuationToken(std::move(resumeToken))
     {
     }
@@ -105,6 +105,30 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
      * @return std::string
      */
     std::string GetResumeToken() const override { return m_continuationToken; }
+
+    /**
+     * @brief Create a #RecoverDeletedKeyOperation from the \p resumeToken fetched from another
+     * `Operation<T>`, updated to the the latest operation status.
+     *
+     * @remark After the operation is initialized, it is used to poll the last update from the
+     * server using the \p context.
+     *
+     * @param resumeToken A previously generated token used to resume the polling of the operation.
+     * @param client A #KeyClient that is used for getting status updates.
+     * @param context A #Azure::Core::Context controlling the request lifetime.
+     * @return DeleteKeyOperation
+     */
+    static RecoverDeletedKeyOperation CreateFromResumeToken(
+        std::string const& resumeToken,
+        Azure::Security::KeyVault::Keys::KeyClient const& client,
+        Azure::Core::Context const& context = Azure::Core::Context())
+    {
+
+      RecoverDeletedKeyOperation operation(
+          resumeToken, std::make_shared<Azure::Security::KeyVault::Keys::KeyClient>(client));
+      operation.Poll(context);
+      return operation;
+    }
   };
 
 }}}} // namespace Azure::Security::KeyVault::Keys

sdk/keyvault/azure-security-keyvault-keys/src/delete_key_operation.cpp

@@ -8,6 +8,7 @@
 #include "azure/keyvault/keys/delete_key_operation.hpp"
 #include "azure/keyvault/keys/details/key_constants.hpp"
 #include "azure/keyvault/keys/details/key_serializers.hpp"
+#include "azure/keyvault/keys/key_client.hpp"
 
 using namespace Azure::Security::KeyVault::Keys;
 using namespace Azure::Security::KeyVault;
@@ -19,8 +20,14 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation::PollInternal(
   std::unique_ptr<Azure::Core::Http::RawResponse> rawResponse;
   if (!IsDone())
   {
-    rawResponse = m_pipeline->Send(
-        context, Azure::Core::Http::HttpMethod::Get, {_detail::DeletedKeysPath, m_value.Name()});
+    try
+    {
+      rawResponse = m_keyClient->GetDeletedKey(m_value.Name(), context).RawResponse;
+    }
+    catch (Azure::Core::RequestFailedException& error)
+    {
+      rawResponse = std::move(error.RawResponse);
+    }
 
     switch (rawResponse->GetStatusCode())
     {
@@ -53,9 +60,9 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation::PollInternal(
 }
 
 Azure::Security::KeyVault::Keys::DeleteKeyOperation::DeleteKeyOperation(
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
+    std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient,
     Azure::Response<Azure::Security::KeyVault::Keys::DeletedKey> response)
-    : m_pipeline(keyvaultPipeline)
+    : m_keyClient(keyClient)
 {
   // The response becomes useless and the value and rawResponse are now owned by the
   // DeleteKeyOperation. This is fine because the DeleteKeyOperation is what the delete key api

sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp

@@ -205,7 +205,7 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation KeyClient::StartDeleteKey(
     Azure::Core::Context const& context) const
 {
   return Azure::Security::KeyVault::Keys::DeleteKeyOperation(
-      m_pipeline,
+      std::make_shared<KeyClient>(*this),
       m_pipeline->SendRequest<Azure::Security::KeyVault::Keys::DeletedKey>(
           context,
           Azure::Core::Http::HttpMethod::Delete,
@@ -215,30 +215,12 @@ Azure::Security::KeyVault::Keys::DeleteKeyOperation KeyClient::StartDeleteKey(
           {_detail::KeysPath, name}));
 }
 
-Azure::Security::KeyVault::Keys::DeleteKeyOperation KeyClient::ResumeDeleteKey(
-    std::string const& resumeToken,
-    Azure::Core::Context const& context) const
-{
-  Azure::Security::KeyVault::Keys::DeleteKeyOperation operation(m_pipeline, resumeToken);
-  operation.Poll(context);
-  return operation;
-}
-
-Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation KeyClient::ResumeRecoverDeletedKey(
-    std::string const& resumeToken,
-    Azure::Core::Context const& context) const
-{
-  Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation operation(m_pipeline, resumeToken);
-  operation.Poll(context);
-  return operation;
-}
-
 Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation KeyClient::StartRecoverDeletedKey(
     std::string const& name,
     Azure::Core::Context const& context) const
 {
   return Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation(
-      m_pipeline,
+      std::make_shared<KeyClient>(*this),
       m_pipeline->SendRequest<Azure::Security::KeyVault::Keys::KeyVaultKey>(
           context,
           Azure::Core::Http::HttpMethod::Post,

sdk/keyvault/azure-security-keyvault-keys/src/recover_deleted_key_operation.cpp

@@ -5,6 +5,7 @@
 
 #include "azure/keyvault/keys/details/key_constants.hpp"
 #include "azure/keyvault/keys/details/key_serializers.hpp"
+#include "azure/keyvault/keys/key_client.hpp"
 #include "azure/keyvault/keys/recover_deleted_key_operation.hpp"
 
 using namespace Azure::Security::KeyVault::Keys;
@@ -17,10 +18,14 @@ Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation::PollInternal(
   std::unique_ptr<Azure::Core::Http::RawResponse> rawResponse;
   if (!IsDone())
   {
-    rawResponse = m_pipeline->Send(
-        context,
-        Azure::Core::Http::HttpMethod::Get,
-        {_detail::KeysPath, m_value.Name(), m_value.Properties.Version});
+    try
+    {
+      rawResponse = m_keyClient->GetKey(m_value.Name(), {}, context).RawResponse;
+    }
+    catch (Azure::Core::RequestFailedException& error)
+    {
+      rawResponse = std::move(error.RawResponse);
+    }
 
     switch (rawResponse->GetStatusCode())
     {
@@ -52,9 +57,9 @@ Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation::PollInternal(
 }
 
 Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation::RecoverDeletedKeyOperation(
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> keyvaultPipeline,
+    std::shared_ptr<Azure::Security::KeyVault::Keys::KeyClient> keyClient,
     Azure::Response<Azure::Security::KeyVault::Keys::KeyVaultKey> response)
-    : m_pipeline(keyvaultPipeline)
+    : m_keyClient(keyClient)
 {
   // The response becomes useless and the value and rawResponse are now owned by the
   // DeleteKeyOperation. This is fine because the DeleteKeyOperation is what the delete key api

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

@@ -337,7 +337,9 @@ TEST_F(KeyVaultClientTest, DeleteOperationResumeToken)
   }
   // Resume operation from token
   {
-    auto resumeOperation = keyClient.ResumeDeleteKey(resumeToken);
+    auto resumeOperation
+        = Azure::Security::KeyVault::Keys::DeleteKeyOperation::CreateFromResumeToken(
+            resumeToken, keyClient);
     resumeOperation.PollUntilDone(std::chrono::milliseconds(500));
   }
   {
@@ -375,7 +377,9 @@ TEST_F(KeyVaultClientTest, RecoverOperationResumeToken)
   }
   // Resume operation from token
   {
-    auto resumeOperation = keyClient.ResumeDeleteKey(resumeToken);
+    auto resumeOperation
+        = Azure::Security::KeyVault::Keys::DeleteKeyOperation::CreateFromResumeToken(
+            resumeToken, keyClient);
     resumeOperation.PollUntilDone(std::chrono::milliseconds(500));
   }
   {
@@ -385,7 +389,9 @@ TEST_F(KeyVaultClientTest, RecoverOperationResumeToken)
   }
   {
     // resume from token
-    auto resumeRecoveryOp = keyClient.ResumeRecoverDeletedKey(resumeToken);
+    auto resumeRecoveryOp
+        = Azure::Security::KeyVault::Keys::RecoverDeletedKeyOperation::CreateFromResumeToken(
+            resumeToken, keyClient);
     auto keyResponse = resumeRecoveryOp.PollUntilDone(std::chrono::milliseconds(500));
     auto key = keyResponse.Value;
     // Delete again for purging