From 417c7fa1b5c11b91938f3e37c761e350c154d746 Mon Sep 17 00:00:00 2001 From: George Arama <50641385+gearama@users.noreply.github.com> Date: Mon, 27 Jun 2022 12:18:00 -0700 Subject: [PATCH] Per request (#3770) * Per request * const * Jeff feedback , clang, and test --- .../certificate_client_options.hpp | 54 +++---------------- .../src/certificate_client.cpp | 7 +-- .../test/ut/certificate_client_test.cpp | 19 +++++++ .../CMakeLists.txt | 1 - .../keyvault/keys/key_client_options.hpp | 50 +++-------------- .../src/key_client.cpp | 2 +- .../src/key_client_options.cpp | 9 ---- .../test/ut/key_client_test.cpp | 8 +-- .../keyvault/secrets/keyvault_options.hpp | 46 +--------------- .../src/secret_client.cpp | 7 +-- .../test/ut/secret_client_test.cpp | 11 ++-- 11 files changed, 47 insertions(+), 167 deletions(-) delete mode 100644 sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp diff --git a/sdk/keyvault/azure-security-keyvault-certificates/inc/azure/keyvault/certificates/certificate_client_options.hpp b/sdk/keyvault/azure-security-keyvault-certificates/inc/azure/keyvault/certificates/certificate_client_options.hpp index 0f7405f7d1..fd38a403bd 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/inc/azure/keyvault/certificates/certificate_client_options.hpp +++ b/sdk/keyvault/azure-security-keyvault-certificates/inc/azure/keyvault/certificates/certificate_client_options.hpp @@ -18,65 +18,23 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Certificates { - /** - * @brief The API version to use from Key Vault. - * - */ - class ServiceVersion final { - private: - std::string m_version; - - public: - /** - * @brief Construct a new Service Version object - * - * @param version The string version for the Key Vault Certificate service. - */ - ServiceVersion(std::string version) : m_version(std::move(version)) {} - - /** - * @brief Enable comparing the ext enum. - * - * @param other Another #ServiceVersion to be compared. - */ - bool operator==(ServiceVersion const& other) const { return m_version == other.m_version; } - - /** - * @brief Return the #ServiceVersion string representation. - * - */ - std::string const& ToString() const { return m_version; } - - /** - * @brief Use to send request to the 7.2 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_CERTIFICATES_DLLEXPORT static const ServiceVersion V7_2; - - /** - * @brief Use to send request to the 7.3 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_CERTIFICATES_DLLEXPORT static const ServiceVersion V7_3; - }; - /** * @brief Define the options to create an SDK Certificate client. * */ struct CertificateClientOptions final : public Azure::Core::_internal::ClientOptions { - ServiceVersion Version; + /** + * @brief Service Version used. + * + */ + std::string Version; /** * @brief Construct a new Certificate Client Options object. * - * @param version Optional version for the client. */ - CertificateClientOptions(ServiceVersion version = ServiceVersion::V7_3) - : Azure::Core::_internal::ClientOptions(), Version(version) - { - } + CertificateClientOptions() : Azure::Core::_internal::ClientOptions() { Version = "7.3"; } }; }}}} // namespace Azure::Security::KeyVault::Certificates diff --git a/sdk/keyvault/azure-security-keyvault-certificates/src/certificate_client.cpp b/sdk/keyvault/azure-security-keyvault-certificates/src/certificate_client.cpp index ad1fb4ce02..fd012dc70e 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/src/certificate_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-certificates/src/certificate_client.cpp @@ -66,9 +66,9 @@ CertificateClient::CertificateClient( std::string const& vaultUrl, std::shared_ptr credential, CertificateClientOptions options) - : m_vaultUrl(vaultUrl), m_apiVersion(options.Version.ToString()) + : m_vaultUrl(vaultUrl), m_apiVersion(options.Version) { - auto apiVersion = options.Version.ToString(); + auto apiVersion = options.Version; std::vector> perRetrypolicies; { @@ -493,6 +493,3 @@ Azure::Response CertificateClient::UpdateCertificatePropert auto value = KeyVaultCertificateSerializer::Deserialize(certificateName, *rawResponse); return Azure::Response(std::move(value), std::move(rawResponse)); } - -const ServiceVersion ServiceVersion::V7_3("7.3"); -const ServiceVersion ServiceVersion::V7_2("7.2"); diff --git a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_test.cpp b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_test.cpp index 6ebd543bb1..f167d7f3f1 100644 --- a/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-certificates/test/ut/certificate_client_test.cpp @@ -888,3 +888,22 @@ TEST_F(KeyVaultCertificateClientTest, DISABLED_MergeCertificate) } } } + +TEST_F(KeyVaultCertificateClientTest, ServiceVersion) +{ + auto credential + = std::make_shared("tenantID", "AppId", "SecretId"); + { + // 7.3 + EXPECT_NO_THROW(auto options = CertificateClientOptions(); CertificateClient certificateClient( + "http://account.vault.azure.net", credential, options); + EXPECT_EQ(options.Version, "7.3");); + } + { + // arbitrary version + EXPECT_NO_THROW( + auto options = CertificateClientOptions(); options.Version = "1.0"; + CertificateClient certificateClient("http://account.vault.azure.net", credential, options); + EXPECT_EQ(options.Version, "1.0");); + } +} \ No newline at end of file diff --git a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt b/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt index c6a2373f6e..570118e046 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt +++ b/sdk/keyvault/azure-security-keyvault-keys/CMakeLists.txt @@ -95,7 +95,6 @@ set( src/json_web_key.cpp src/key_backup.cpp src/key_client.cpp - src/key_client_options.cpp src/key_client_paged_responses.cpp src/key_curve_name.cpp src/key_operation.cpp diff --git a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp index 334954c39c..1fe104d7a4 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp +++ b/sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/key_client_options.hpp @@ -43,61 +43,23 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { Azure::Nullable NextPageToken; }; - class ServiceVersion final { - private: - std::string m_version; - - public: - /** - * @brief Construct a new Service Version object - * - * @param version The string version for the Key Vault keys service. - */ - ServiceVersion(std::string version) : m_version(std::move(version)) {} - - /** - * @brief Enable comparing the ext enum. - * - * @param other Another #ServiceVersion to be compared. - */ - bool operator==(ServiceVersion const& other) const { return m_version == other.m_version; } - - /** - * @brief Return the #ServiceVersion string representation. - * - */ - std::string const& ToString() const { return m_version; } - - /** - * @brief Use to send request to the 7.2 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_2; - - /** - * @brief Use to send request to the 7.3 version of Key Vault service. - * - */ - AZ_SECURITY_KEYVAULT_KEYS_DLLEXPORT static const ServiceVersion V7_3; - }; - /** * @brief Define the options to create an SDK Keys client. * */ struct KeyClientOptions final : public Azure::Core::_internal::ClientOptions { - ServiceVersion Version; + /** + * @brief Service Version used. + * + */ + std::string Version; /** * @brief Construct a new Key Client Options object. * - * @param version Optional version for the client. */ - KeyClientOptions(ServiceVersion version = ServiceVersion::V7_3) - : Azure::Core::_internal::ClientOptions(), Version(version) - { - } + KeyClientOptions() : Azure::Core::_internal::ClientOptions() { Version = "7.3"; } }; /** 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 230ecb2428..a098c63d72 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/src/key_client.cpp @@ -68,7 +68,7 @@ KeyClient::KeyClient( std::string const& vaultUrl, std::shared_ptr credential, KeyClientOptions options) - : m_vaultUrl(vaultUrl), m_apiVersion(options.Version.ToString()) + : m_vaultUrl(vaultUrl), m_apiVersion(options.Version) { std::vector> perRetrypolicies; { diff --git a/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp b/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp deleted file mode 100644 index 2571e46609..0000000000 --- a/sdk/keyvault/azure-security-keyvault-keys/src/key_client_options.cpp +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// SPDX-License-Identifier: MIT - -#include "azure/keyvault/keys/key_client_options.hpp" - -namespace Azure { namespace Security { namespace KeyVault { namespace Keys { - const ServiceVersion ServiceVersion::V7_3("7.3"); - const ServiceVersion ServiceVersion::V7_2("7.2"); -}}}} // namespace Azure::Security::KeyVault::Keys diff --git a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp index 46065f2f04..effe82b554 100644 --- a/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-keys/test/ut/key_client_test.cpp @@ -34,15 +34,15 @@ TEST(KeyVaultKeyClientUnitTest, ServiceVersion) = std::make_shared("tenantID", "AppId", "SecretId"); { // 7.3 - EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion::V7_3); + EXPECT_NO_THROW(auto options = KeyClientOptions(); KeyClient keyClient("http://account.vault.azure.net", credential, options); - EXPECT_EQ(options.Version.ToString(), "7.3");); + EXPECT_EQ(options.Version, "7.3");); } { // arbitrary version - EXPECT_NO_THROW(auto options = KeyClientOptions(ServiceVersion("1.0")); + EXPECT_NO_THROW(auto options = KeyClientOptions(); options.Version = "1.0"; KeyClient keyClient("http://account.vault.azure.net", credential, options); - EXPECT_EQ(options.Version.ToString(), "1.0");); + EXPECT_EQ(options.Version, "1.0");); } } diff --git a/sdk/keyvault/azure-security-keyvault-secrets/inc/azure/keyvault/secrets/keyvault_options.hpp b/sdk/keyvault/azure-security-keyvault-secrets/inc/azure/keyvault/secrets/keyvault_options.hpp index 01d865ec12..fa08daf835 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/inc/azure/keyvault/secrets/keyvault_options.hpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/inc/azure/keyvault/secrets/keyvault_options.hpp @@ -11,44 +11,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets { - class ServiceVersion final { - private: - std::string m_version; - - public: - /** - * @brief Construct a new Service Version object - * - * @param version The string version for the Key Vault keys service. - */ - ServiceVersion(std::string version) : m_version(std::move(version)) {} - - /** - * @brief Enable comparing the ext enum. - * - * @param other Another #ServiceVersion to be compared. - */ - bool operator==(ServiceVersion const& other) const { return m_version == other.m_version; } - - /** - * @brief Return the #ServiceVersion string representation. - * - */ - std::string const& ToString() const { return m_version; } - - /** - * @brief Use to send request to the 7.2 version of Key Vault service. - * - */ - AZURE_SECURITY_KEYVAULT_SECRETS_DLLEXPORT static const ServiceVersion V7_2; - - /** - * @brief Use to send request to the 7.3 version of Key Vault service. - * - */ - AZURE_SECURITY_KEYVAULT_SECRETS_DLLEXPORT static const ServiceVersion V7_3; - }; - /** * @brief Define the options to create an SDK Keys client. * @@ -59,17 +21,13 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets { * @brief Service Version used. * */ - const ServiceVersion Version; + std::string Version; /** * @brief Construct a new Secret Client Options object. * - * @param version Optional version for the client. */ - SecretClientOptions(ServiceVersion version = ServiceVersion::V7_3) - : Azure::Core::_internal::ClientOptions(), Version(version) - { - } + SecretClientOptions() : Azure::Core::_internal::ClientOptions() { Version = "7.3"; } }; /** diff --git a/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp b/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp index bbceb91328..194a9df4df 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/src/secret_client.cpp @@ -27,9 +27,6 @@ using namespace Azure::Core::Http::Policies; using namespace Azure::Core::Http::Policies::_internal; using namespace Azure::Security::KeyVault::Secrets::_detail; -const ServiceVersion ServiceVersion::V7_3("7.3"); -const ServiceVersion ServiceVersion::V7_2("7.2"); - std::unique_ptr SecretClient::SendRequest( Azure::Core::Http::Request& request, Azure::Core::Context const& context) const @@ -64,9 +61,9 @@ SecretClient::SecretClient( std::string const& vaultUrl, std::shared_ptr credential, SecretClientOptions options) - : m_vaultUrl(vaultUrl), m_apiVersion(options.Version.ToString()) + : m_vaultUrl(vaultUrl), m_apiVersion(options.Version) { - auto apiVersion = options.Version.ToString(); + auto apiVersion = options.Version; Azure::Core::Url url(vaultUrl); std::vector> perRetrypolicies; diff --git a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp index ba4b3c3886..6358f70d97 100644 --- a/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp +++ b/sdk/keyvault/azure-security-keyvault-secrets/test/ut/secret_client_test.cpp @@ -30,17 +30,16 @@ TEST(SecretClient, ServiceVersion) = std::make_shared("tenantID", "AppId", "SecretId"); { // 7.3 - EXPECT_NO_THROW( - auto options = SecretClientOptions(ServiceVersion::V7_3); - SecretClient SecretClient("http://account.vault.azure.net", credential, options); - EXPECT_EQ(options.Version.ToString(), "7.3");); + EXPECT_NO_THROW(auto options = SecretClientOptions(); SecretClient SecretClient( + "http://account.vault.azure.net", credential, options); + EXPECT_EQ(options.Version, "7.3");); } { // arbitrary version EXPECT_NO_THROW( - auto options = SecretClientOptions(ServiceVersion("1.0")); + auto options = SecretClientOptions(); options.Version = "1.0"; SecretClient secretClient("http://account.vault.azure.net", credential, options); - EXPECT_EQ(options.Version.ToString(), "1.0");); + EXPECT_EQ(options.Version, "1.0");); } }