From c4ff48e8a52b5523c1d1122216bb2f621f8c268d Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Wed, 14 Jul 2021 12:42:13 -0700 Subject: [PATCH] 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 | 1 - .../CMakeLists.txt | 11 ++------ .../test/ut/CMakeLists.txt | 3 +-- .../CMakeLists.txt | 11 +++++--- .../keys/cryptography/cryptography_client.hpp | 4 +-- .../remote_cryptography_client.hpp | 4 +-- .../inc/azure/keyvault/keys/key_client.hpp | 4 +-- .../remote_cryptography_client.cpp | 2 +- .../src/key_client.cpp | 2 +- .../src/keyvault_protocol.cpp} | 27 +++++-------------- .../src/private/keyvault_constants.hpp | 0 .../src/private/keyvault_protocol.hpp} | 12 ++++----- .../test/ut/CMakeLists.txt | 1 + .../test/ut/key_client_import_test_live.cpp | 2 +- .../test/ut/mocked_transport_adapter_test.hpp | 2 +- .../test/ut/protocol_test.cpp} | 9 ++++--- sdk/keyvault/ci.yml | 3 --- 17 files changed, 39 insertions(+), 59 deletions(-) rename sdk/keyvault/{azure-security-keyvault-common/src/keyvault_pipeline.cpp => azure-security-keyvault-keys/src/keyvault_protocol.cpp} (68%) rename sdk/keyvault/{azure-security-keyvault-common => azure-security-keyvault-keys}/src/private/keyvault_constants.hpp (100%) rename sdk/keyvault/{azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp => azure-security-keyvault-keys/src/private/keyvault_protocol.hpp} (96%) rename sdk/keyvault/{azure-security-keyvault-common/test/ut/pipeline_test.cpp => azure-security-keyvault-keys/test/ut/protocol_test.cpp} (74%) diff --git a/sdk/keyvault/CMakeLists.txt b/sdk/keyvault/CMakeLists.txt index 9d62942a1..4a6420ffa 100644 --- a/sdk/keyvault/CMakeLists.txt +++ b/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) diff --git a/sdk/keyvault/azure-security-keyvault-common/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-common/CMakeLists.txt index 34e1e13a6..cfdd0c356 100644 --- a/sdk/keyvault/azure-security-keyvault-common/CMakeLists.txt +++ b/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 diff --git a/sdk/keyvault/azure-security-keyvault-common/test/ut/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-common/test/ut/CMakeLists.txt index f43abae24..0efaa0175 100644 --- a/sdk/keyvault/azure-security-keyvault-common/test/ut/CMakeLists.txt +++ b/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. diff --git a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt index beaf4eca1..9c9cfb4c5 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt +++ b/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( $ ) -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) diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp index d48fadce3..3bb29c8a2 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/cryptography/cryptography_client.hpp @@ -9,7 +9,7 @@ #pragma once -#include +#include "../src/private/keyvault_protocol.hpp" #include @@ -36,7 +36,7 @@ namespace Azure { */ class CryptographyClient final { private: - std::shared_ptr m_pipeline; + std::shared_ptr m_pipeline; std::string m_keyId; std::shared_ptr< Azure::Security::KeyVault::Keys::Cryptography::_detail::RemoteCryptographyClient> diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/internal/cryptography/remote_cryptography_client.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/internal/cryptography/remote_cryptography_client.hpp index a7a4b2934..e2de3527a 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/internal/cryptography/remote_cryptography_client.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/internal/cryptography/remote_cryptography_client.hpp @@ -12,7 +12,7 @@ #include #include -#include +#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 Pipeline; + std::shared_ptr Pipeline; Azure::Core::Url KeyId; explicit RemoteCryptographyClient( diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client.hpp index 19930861e..6d3d87baf 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client.hpp @@ -9,7 +9,7 @@ #pragma once -#include +#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 m_pipeline; + std::shared_ptr m_pipeline; public: /** diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/remote_cryptography_client.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/remote_cryptography_client.cpp index 294fef36a..c5ad461a0 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/remote_cryptography_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/cryptography/remote_cryptography_client.cpp @@ -41,7 +41,7 @@ RemoteCryptographyClient::RemoteCryptographyClient( std::make_unique(credential, tokenContext)); } - Pipeline = std::make_shared( + Pipeline = std::make_shared( Azure::Core::Url(keyId), apiVersion, Azure::Core::Http::_internal::HttpPipeline( diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp index aef67191b..d0536e60a 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp @@ -103,7 +103,7 @@ KeyClient::KeyClient( std::make_unique(credential, tokenContext)); } - m_pipeline = std::make_shared( + m_pipeline = std::make_shared( Azure::Core::Url(vaultUrl), apiVersion, Azure::Core::Http::_internal::HttpPipeline( diff --git a/sdk/keyvault/azure-security-keyvault-common/src/keyvault_pipeline.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/keyvault_protocol.cpp similarity index 68% rename from sdk/keyvault/azure-security-keyvault-common/src/keyvault_pipeline.cpp rename to sdk/keyvault/azure-security-keyvault-keys/src/keyvault_protocol.cpp index 836e36e48..bf0160156 100644 --- a/sdk/keyvault/azure-security-keyvault-common/src/keyvault_pipeline.cpp +++ b/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 @@ -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 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 const& path) const { return CreateRequest(method, nullptr, path); } -std::unique_ptr _internal::KeyVaultPipeline::SendRequest( +std::unique_ptr _detail::KeyVaultProtocolClient::SendRequest( Azure::Core::Context const& context, Azure::Core::Http::Request& request) const { diff --git a/sdk/keyvault/azure-security-keyvault-common/src/private/keyvault_constants.hpp b/sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_constants.hpp similarity index 100% rename from sdk/keyvault/azure-security-keyvault-common/src/private/keyvault_constants.hpp rename to sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_constants.hpp diff --git a/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp b/sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_protocol.hpp similarity index 96% rename from sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp rename to sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_protocol.hpp index a27b09eb6..c4e13b683 100644 --- a/sdk/keyvault/azure-security-keyvault-common/inc/azure/keyvault/common/internal/keyvault_pipeline.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/private/keyvault_protocol.hpp @@ -21,13 +21,13 @@ #include #include -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 diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-keys/test/ut/CMakeLists.txt index 2f825c2f9..b7e9b92fa 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/CMakeLists.txt +++ b/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) diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_import_test_live.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_import_test_live.cpp index 9b487433a..ab351503e 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_import_test_live.cpp +++ b/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) { diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_transport_adapter_test.hpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_transport_adapter_test.hpp index 7c8165a0f..2b76974d0 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/mocked_transport_adapter_test.hpp +++ b/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( + m_pipeline = std::make_unique( Azure::Core::Url(vaultUrl), apiVersion, Azure::Core::Http::_internal::HttpPipeline(options, "test", "version", {}, {})); diff --git a/sdk/keyvault/azure-security-keyvault-common/test/ut/pipeline_test.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/protocol_test.cpp similarity index 74% rename from sdk/keyvault/azure-security-keyvault-common/test/ut/pipeline_test.cpp rename to sdk/keyvault/azure-security-keyvault-keys/test/ut/protocol_test.cpp index 823dcd9f5..23f42cd7f 100644 --- a/sdk/keyvault/azure-security-keyvault-common/test/ut/pipeline_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/protocol_test.cpp @@ -6,13 +6,14 @@ #include #include #include -#include + +#include "./../../src/private/keyvault_protocol.hpp" #include -using namespace Azure::Security::KeyVault::_internal; +using namespace Azure::Security::KeyVault::_detail; -TEST(KeyVaultPipeline, initPipeline) +TEST(KeyVaultProtocolClient, initPipeline) { std::vector> 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))); } diff --git a/sdk/keyvault/ci.yml b/sdk/keyvault/ci.yml index 3f426ebd4..74e405345 100644 --- a/sdk/keyvault/ci.yml +++ b/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