Rename KeyVaultPipeline to KeyVaultProtocolClient and make it an impl detail (move it to _detail) within the keys package instead of common (#2618)

* Remove InitRequest helper method

* Rename KeyVaultPipeline to KeyVaultProtocolClient along with headers.

* Move the KeyVaultProtocolClient into private imlpementation detail of
Keys.

* Update doc comments.

* Stop building keyvault common for now since it contains no sources.

* Don't generate keyvault common docs since there are no files.

* Stop building azure-security-keyvault-common as part of CI.

sdk/keyvault/CMakeLists.txt

@@ -12,5 +12,4 @@ if(BUILD_TESTING)
   add_compile_definitions(TESTING_BUILD)
 endif()
 
-add_subdirectory(azure-security-keyvault-common)
 add_subdirectory(azure-security-keyvault-keys)

sdk/keyvault/azure-security-keyvault-common/CMakeLists.txt

@@ -26,21 +26,14 @@ if(NOT AZ_ALL_LIBRARIES)
   endif()
 endif()
 
-set(
-  AZURE_KEYVAULT_COMMON_HEADER
-    inc/azure/keyvault/common/internal/keyvault_pipeline.hpp
-)
-
 set(
   AZURE_KEYVAULT_COMMON_SOURCE
-    src/private/keyvault_constants.hpp
     src/private/package_version.hpp
-    src/keyvault_pipeline.cpp
 )
 
 add_library(
     azure-security-keyvault-common
-    ${AZURE_KEYVAULT_COMMON_HEADER} ${AZURE_KEYVAULT_COMMON_SOURCE}
+    ${AZURE_KEYVAULT_COMMON_SOURCE}
 )
 add_library(Azure::azure-security-keyvault-common ALIAS azure-security-keyvault-common)
 
@@ -69,7 +62,7 @@ endif()
 create_code_coverage(keyvault azure-security-keyvault-common azure-security-keyvault-common-test)
 
 get_az_version("${CMAKE_CURRENT_SOURCE_DIR}/src/private/package_version.hpp")
-generate_documentation(azure-security-keyvault-common ${AZ_LIBRARY_VERSION})
+#generate_documentation(azure-security-keyvault-common ${AZ_LIBRARY_VERSION})
 
 if(BUILD_TESTING)
     # tests

sdk/keyvault/azure-security-keyvault-common/test/ut/CMakeLists.txt

@@ -12,14 +12,13 @@ include(GoogleTest)
 add_executable (
   azure-security-keyvault-common-test
     azure_security_keyvault_common_test.cpp
-    pipeline_test.cpp
 )
 
 if (MSVC)
   target_compile_options(azure-security-keyvault-common-test PUBLIC /wd6326 /wd26495 /wd26812)
 endif()
 
-target_link_libraries(azure-security-keyvault-common-test PUBLIC azure-security-keyvault-common gtest gmock)
+target_link_libraries(azure-security-keyvault-common-test PUBLIC gtest gmock)
 
 # gtest_discover_tests will scan the test from azure-core-test and call add_test
 # for each test to ctest. This enables `ctest -r` to run specific tests directly.

sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt

@@ -20,9 +20,9 @@ include(AzureGlobalCompileOptions)
 az_vcpkg_integrate()
 
 if(NOT AZ_ALL_LIBRARIES)
-  find_package(azure-security-keyvault-common-cpp "4.0.0" CONFIG QUIET)
-  if(NOT azure-security-keyvault-common-cpp_FOUND)
-    find_package(azure-security-keyvault-common-cpp "4.0.0" REQUIRED)
+  find_package(azure-core-cpp "1.1.0" CONFIG QUIET)
+  if(NOT azure-core-cpp_FOUND)
+    find_package(azure-core-cpp "1.1.0" REQUIRED)
   endif()
 endif()
 
@@ -91,6 +91,8 @@ set(
     src/private/key_sign_parameters.hpp
     src/private/key_verify_parameters.hpp
     src/private/key_wrap_parameters.hpp
+    src/private/keyvault_constants.hpp
+    src/private/keyvault_protocol.hpp
     src/private/package_version.hpp
     src/delete_key_operation.cpp
     src/deleted_key.cpp
@@ -104,6 +106,7 @@ set(
     src/key_request_parameters.cpp
     src/key_type.cpp
     src/key_vault_key.cpp
+    src/keyvault_protocol.cpp
     src/list_keys_responses.cpp
     src/recover_deleted_key_operation.cpp
 )
@@ -120,7 +123,7 @@ target_include_directories(
       $<INSTALL_INTERFACE:include>
 )
 
-target_link_libraries(azure-security-keyvault-keys PUBLIC Azure::azure-security-keyvault-common)
+target_link_libraries(azure-security-keyvault-keys PUBLIC Azure::azure-core)
 
 # coverage. Has no effect if BUILD_CODE_COVERAGE is OFF
 create_code_coverage(keyvault azure-security-keyvault-keys azure-security-keyvault-keys-test)

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp

@@ -9,7 +9,7 @@
 
 #pragma once
 
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
+#include "../src/private/keyvault_protocol.hpp"
 
 #include <azure/core/io/body_stream.hpp>
 
@@ -36,7 +36,7 @@ namespace Azure {
    */
   class CryptographyClient final {
   private:
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> m_pipeline;
+    std::shared_ptr<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient> m_pipeline;
     std::string m_keyId;
     std::shared_ptr<
         Azure::Security::KeyVault::Keys::Cryptography::_detail::RemoteCryptographyClient>

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/internal/cryptography/remote_cryptography_client.hpp

@@ -12,7 +12,7 @@
 #include <azure/core/response.hpp>
 #include <azure/core/url.hpp>
 
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
+#include "../src/private/keyvault_protocol.hpp"
 
 #include "azure/keyvault/keys/cryptography/cryptography_client_options.hpp"
 #include "azure/keyvault/keys/cryptography/encrypt_parameters.hpp"
@@ -34,7 +34,7 @@ namespace Azure {
         : public Azure::Security::KeyVault::Keys::Cryptography::_detail::CryptographyProvider
     {
 
-      std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> Pipeline;
+      std::shared_ptr<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient> Pipeline;
       Azure::Core::Url KeyId;
 
       explicit RemoteCryptographyClient(

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

@@ -9,7 +9,7 @@
 
 #pragma once
 
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
+#include "../src/private/keyvault_protocol.hpp"
 
 #include "azure/keyvault/keys/backup_key_result.hpp"
 #include "azure/keyvault/keys/delete_key_operation.hpp"
@@ -60,7 +60,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys {
   {
   protected:
     // Using a shared pipeline for a client to share it with LRO (like delete key)
-    std::shared_ptr<Azure::Security::KeyVault::_internal::KeyVaultPipeline> m_pipeline;
+    std::shared_ptr<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient> m_pipeline;
 
   public:
     /**

sdk/keyvault/azure-security-keyvault-keys/src/cryptography/remote_cryptography_client.cpp

@@ -41,7 +41,7 @@ RemoteCryptographyClient::RemoteCryptographyClient(
         std::make_unique<BearerTokenAuthenticationPolicy>(credential, tokenContext));
   }
 
-  Pipeline = std::make_shared<Azure::Security::KeyVault::_internal::KeyVaultPipeline>(
+  Pipeline = std::make_shared<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient>(
       Azure::Core::Url(keyId),
       apiVersion,
       Azure::Core::Http::_internal::HttpPipeline(

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

@@ -103,7 +103,7 @@ KeyClient::KeyClient(
         std::make_unique<BearerTokenAuthenticationPolicy>(credential, tokenContext));
   }
 
-  m_pipeline = std::make_shared<Azure::Security::KeyVault::_internal::KeyVaultPipeline>(
+  m_pipeline = std::make_shared<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient>(
       Azure::Core::Url(vaultUrl),
       apiVersion,
       Azure::Core::Http::_internal::HttpPipeline(

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

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-#include "azure/keyvault/common/internal/keyvault_pipeline.hpp"
+#include "private/keyvault_protocol.hpp"
 #include "private/keyvault_constants.hpp"
 
 #include <azure/core/exception.hpp>
@@ -10,27 +10,14 @@
 using namespace Azure::Security::KeyVault;
 using namespace Azure::Core::Http::_internal;
 
-namespace {
-inline Azure::Core::Http::Request InitRequest(
-    Azure::Core::Http::HttpMethod method,
-    Azure::Core::IO::BodyStream* content,
-    Azure::Core::Url const& url)
-{
-  if (content == nullptr)
-  {
-    return Azure::Core::Http::Request(method, url);
-  }
-  return Azure::Core::Http::Request(method, url, content);
-}
-} // namespace
-
-Azure::Core::Http::Request _internal::KeyVaultPipeline::CreateRequest(
+Azure::Core::Http::Request _detail::KeyVaultProtocolClient::CreateRequest(
     Azure::Core::Http::HttpMethod method,
     Azure::Core::IO::BodyStream* content,
     std::vector<std::string> const& path) const
 {
-
-  auto request = ::InitRequest(method, content, m_vaultUrl);
+  Azure::Core::Http::Request request = content == nullptr
+      ? Azure::Core::Http::Request(method, m_vaultUrl)
+      : Azure::Core::Http::Request(method, m_vaultUrl, content);
 
   request.SetHeader(HttpShared::ContentType, HttpShared::ApplicationJson);
   request.SetHeader(HttpShared::Accept, HttpShared::ApplicationJson);
@@ -47,14 +34,14 @@ Azure::Core::Http::Request _internal::KeyVaultPipeline::CreateRequest(
   return request;
 }
 
-Azure::Core::Http::Request _internal::KeyVaultPipeline::CreateRequest(
+Azure::Core::Http::Request _detail::KeyVaultProtocolClient::CreateRequest(
     Azure::Core::Http::HttpMethod method,
     std::vector<std::string> const& path) const
 {
   return CreateRequest(method, nullptr, path);
 }
 
-std::unique_ptr<Azure::Core::Http::RawResponse> _internal::KeyVaultPipeline::SendRequest(
+std::unique_ptr<Azure::Core::Http::RawResponse> _detail::KeyVaultProtocolClient::SendRequest(
     Azure::Core::Context const& context,
     Azure::Core::Http::Request& request) const
 {

sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_protocol.hpp

@@ -21,13 +21,13 @@
 #include <string>
 #include <vector>
 
-namespace Azure { namespace Security { namespace KeyVault { namespace _internal {
+namespace Azure { namespace Security { namespace KeyVault { namespace _detail {
 
   /**
-   * @brief The HTTP pipeline used by Key Vault clients.
+   * @brief The Protocol layer used by Key Vault clients.
    *
    */
-  class KeyVaultPipeline final {
+  class KeyVaultProtocolClient final {
     Azure::Core::Url m_vaultUrl;
     Azure::Core::Http::_internal::HttpPipeline m_pipeline;
     std::string m_apiVersion;
@@ -69,13 +69,13 @@ namespace Azure { namespace Security { namespace KeyVault { namespace _internal
 
   public:
     /**
-     * @brief Construct a new Key Vault Pipeline.
+     * @brief Construct a new Key Vault Protocol Client.
      *
      * @param vaultUrl The URL address for the Key Vault.
      * @param apiVersion The service API version.
      * @param pipeline The HTTP pipeline for sending requests with.
      */
-    explicit KeyVaultPipeline(
+    explicit KeyVaultProtocolClient(
         Azure::Core::Url vaultUrl,
         std::string apiVersion,
         Azure::Core::Http::_internal::HttpPipeline&& pipeline)
@@ -187,4 +187,4 @@ namespace Azure { namespace Security { namespace KeyVault { namespace _internal
       return m_pipeline.Send(request, context);
     }
   };
-}}}} // namespace Azure::Security::KeyVault::_internal
+}}}} // namespace Azure::Security::KeyVault::_detail

sdk/keyvault/azure-security-keyvault-keys/test/ut/CMakeLists.txt

@@ -18,6 +18,7 @@ add_executable (
     macro_guard.cpp
     mocked_transport_adapter_test.hpp
     mocked_client_test.cpp
+    protocol_test.cpp
 )
 
 if (MSVC)

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

@@ -20,7 +20,7 @@
 using namespace Azure::Core::_internal;
 using namespace Azure::Security::KeyVault::Keys::Test;
 using namespace Azure::Security::KeyVault::Keys;
-using namespace Azure::Security::KeyVault::_internal;
+using namespace Azure::Security::KeyVault::_detail;
 
 TEST_F(KeyVaultClientTest, ImportKey)
 {

sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_transport_adapter_test.hpp

@@ -101,7 +101,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { nam
     {
       auto apiVersion = options.Version.ToString();
 
-      m_pipeline = std::make_unique<Azure::Security::KeyVault::_internal::KeyVaultPipeline>(
+      m_pipeline = std::make_unique<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient>(
           Azure::Core::Url(vaultUrl),
           apiVersion,
           Azure::Core::Http::_internal::HttpPipeline(options, "test", "version", {}, {}));

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

@@ -6,13 +6,14 @@
 #include <azure/core/http/http.hpp>
 #include <azure/core/http/policies/policy.hpp>
 #include <azure/core/internal/client_options.hpp>
-#include <azure/keyvault/common/internal/keyvault_pipeline.hpp>
+
+#include "./../../src/private/keyvault_protocol.hpp"
 
 #include <memory>
 
-using namespace Azure::Security::KeyVault::_internal;
+using namespace Azure::Security::KeyVault::_detail;
 
-TEST(KeyVaultPipeline, initPipeline)
+TEST(KeyVaultProtocolClient, initPipeline)
 {
   std::vector<std::unique_ptr<Azure::Core::Http::Policies::HttpPolicy>> policies;
   policies.emplace_back(
@@ -21,5 +22,5 @@ TEST(KeyVaultPipeline, initPipeline)
   Azure::Core::_internal::ClientOptions options;
   Azure::Core::Http::_internal::HttpPipeline pipeline(
       options, "service-name", "service-version", std::move(policies), {});
-  EXPECT_NO_THROW(KeyVaultPipeline p(url, "version", std::move(pipeline)));
+  EXPECT_NO_THROW(KeyVaultProtocolClient p(url, "version", std::move(pipeline)));
 }

sdk/keyvault/ci.yml

@@ -38,9 +38,6 @@ stages:
       LiveTestTimeoutInMinutes: 120
       SubscriptionConfiguration: $(sub-config-azure-cloud-test-resources)
       Artifacts:
-        - Name: azure-security-keyvault-common
-          Path: azure-security-keyvault-common
-          VcpkgPortName: azure-security-keyvault-common-cpp
         - Name: azure-security-keyvault-keys
           Path: azure-security-keyvault-keys
           VcpkgPortName: azure-security-keyvault-keys-cpp