Api review feedback (#2795)

* first set of changes

* another set

* add new constructor

* format files, remove pedantinc ;

* factory

* format

* expect_death not behaving as expected
This commit is contained in:
George Arama 2021-08-26 10:45:36 -07:00 committed by GitHub
parent 3ddd7f65f8
commit 04f4d3a5cf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 92 additions and 40 deletions

View File

@ -51,6 +51,7 @@ set(
src/secret_serializers.cpp
src/keyvault_operations.cpp
src/keyvault_secret_paged_response.cpp
src/keyvault_secret_properties.cpp
)
add_library(azure-security-keyvault-secrets ${AZURE_SECURITY_KEYVAULT_SECRETS_HEADER} ${AZURE_SECURITY_KEYVAULT_SECRETS_SOURCE})

View File

@ -26,12 +26,12 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
/**
* @brief The time when the secret is scheduled to be purged, in UTC.
*/
Azure::DateTime ScheduledPurgeDate;
Azure::Nullable<Azure::DateTime> ScheduledPurgeDate;
/**
* @brief The time when the secret was deleted, in UTC.
*/
Azure::DateTime DeletedOn;
Azure::Nullable<Azure::DateTime> DeletedOn;
/**
* @brief Default constructor.

View File

@ -20,15 +20,15 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
/**
* @brief Represents a long running operation to restore a deleted secret.
*/
class RecoverDeletedSecretOperation final : public Azure::Core::Operation<KeyVaultSecret> {
class RecoverDeletedSecretOperation final : public Azure::Core::Operation<SecretProperties> {
private:
friend class SecretClient;
std::shared_ptr<SecretClient> m_secretClient;
KeyVaultSecret m_value;
SecretProperties m_value;
std::string m_continuationToken;
Azure::Response<KeyVaultSecret> PollUntilDoneInternal(
Azure::Response<SecretProperties> PollUntilDoneInternal(
std::chrono::milliseconds period,
Azure::Core::Context& context) override;
@ -43,7 +43,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
*/
RecoverDeletedSecretOperation(
std::shared_ptr<SecretClient> secretClient,
Azure::Response<KeyVaultSecret> response);
Azure::Response<SecretProperties> response);
RecoverDeletedSecretOperation(
std::string resumeToken,
@ -65,7 +65,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
*
* @return A Secret object.
*/
KeyVaultSecret Value() const override { return m_value; }
SecretProperties Value() const override { return m_value; }
/**
* @brief Get an Url as string which can be used to get the status of the operation.

View File

@ -42,18 +42,6 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
*/
KeyVaultSecret() = default;
/**
* @brief The vault url of the secret.
*
*/
std::string VaultUrl() { return Properties.VaultUrl; }
/**
* @brief The version of the secret.
*
*/
std::string Version() { return Properties.Version; }
/**
* @brief Construct a new Secret object.
*

View File

@ -10,7 +10,7 @@
#include <azure/core/datetime.hpp>
#include <azure/core/nullable.hpp>
#include <azure/core/url.hpp>
#include <unordered_map>
namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
@ -134,5 +134,11 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
throw std::invalid_argument("Name cannot be empty");
}
};
/**
* @brief Construct a new secret Properties object.
*
*/
static SecretProperties CreateFromURL(std::string const& url);
};
}}}} // namespace Azure::Security::KeyVault::Secrets

View File

@ -46,7 +46,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
#endif
{
protected:
private:
// Using a shared pipeline for a client to share it with LRO (like delete key)
std::shared_ptr<Azure::Security::KeyVault::_detail::KeyVaultProtocolClient> m_protocolClient;
@ -173,7 +173,7 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
* @return The Secret wrapped in the Response.
*/
Azure::Response<KeyVaultSecret> RestoreSecretBackup(
std::vector<uint8_t> const& backup,
BackupSecretResult const& backup,
Azure::Core::Context const& context = Azure::Core::Context()) const;
/**

View File

@ -10,7 +10,7 @@
#include "azure/keyvault/secrets/secret_client.hpp"
#include "private/secret_serializers.hpp"
Azure::Response<KeyVaultSecret> RecoverDeletedSecretOperation::PollUntilDoneInternal(
Azure::Response<SecretProperties> RecoverDeletedSecretOperation::PollUntilDoneInternal(
std::chrono::milliseconds period,
Azure::Core::Context& context)
{
@ -25,7 +25,7 @@ Azure::Response<KeyVaultSecret> RecoverDeletedSecretOperation::PollUntilDoneInte
std::this_thread::sleep_for(period);
}
return Azure::Response<KeyVaultSecret>(
return Azure::Response<SecretProperties>(
m_value, std::make_unique<Azure::Core::Http::RawResponse>(*m_rawResponse));
}
@ -60,7 +60,8 @@ std::unique_ptr<Azure::Core::Http::RawResponse> RecoverDeletedSecretOperation::P
if (m_status == Azure::Core::OperationStatus::Succeeded)
{
m_value = _detail::SecretSerializer::Deserialize(m_value.Name, *rawResponse);
auto receivedSecret = _detail::SecretSerializer::Deserialize(m_value.Name, *rawResponse);
m_value = receivedSecret.Properties;
}
return rawResponse;
@ -68,7 +69,7 @@ std::unique_ptr<Azure::Core::Http::RawResponse> RecoverDeletedSecretOperation::P
RecoverDeletedSecretOperation::RecoverDeletedSecretOperation(
std::shared_ptr<SecretClient> secretClient,
Azure::Response<KeyVaultSecret> response)
Azure::Response<SecretProperties> response)
: m_secretClient(secretClient)
{
m_value = response.Value;

View File

@ -0,0 +1,22 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// SPDX-License-Identifier: MIT
/**
* @file
* @brief Declares SecretProperties.
*
*/
#include "azure/keyvault/secrets/keyvault_secret.hpp"
#include "private/secret_serializers.hpp"
using namespace Azure::Security::KeyVault::Secrets;
SecretProperties SecretProperties::CreateFromURL(std::string const& url)
{
// create a url object to validate the string is valid as url
Azure::Core::Url urlInstance(url);
SecretProperties result;
// parse the url into the result object
_detail::SecretSerializer::ParseIDUrl(result, urlInstance.GetAbsoluteUrl());
return result;
}

View File

@ -157,13 +157,13 @@ Azure::Response<BackupSecretResult> SecretClient::BackupSecret(
}
Azure::Response<KeyVaultSecret> SecretClient::RestoreSecretBackup(
std::vector<uint8_t> const& backup,
BackupSecretResult const& backup,
Azure::Core::Context const& context) const
{
return m_protocolClient->SendRequest<KeyVaultSecret>(
context,
Azure::Core::Http::HttpMethod::Post,
[&backup]() { return _detail::RestoreSecretSerializer::Serialize(backup); },
[&backup]() { return _detail::RestoreSecretSerializer::Serialize(backup.Secret); },
[](Azure::Core::Http::RawResponse const& rawResponse) {
return _detail::SecretSerializer::Deserialize(rawResponse);
},
@ -201,11 +201,12 @@ Azure::Security::KeyVault::Secrets::RecoverDeletedSecretOperation SecretClient::
{
return Azure::Security::KeyVault::Secrets::RecoverDeletedSecretOperation(
std::make_shared<SecretClient>(*this),
m_protocolClient->SendRequest<KeyVaultSecret>(
m_protocolClient->SendRequest<SecretProperties>(
context,
Azure::Core::Http::HttpMethod::Post,
[&name](Azure::Core::Http::RawResponse const& rawResponse) {
return _detail::SecretSerializer::Deserialize(name, rawResponse);
auto parsedResponse = _detail::SecretSerializer::Deserialize(name, rawResponse);
return parsedResponse.Properties;
},
{_detail::DeletedSecretPath, name, _detail::RecoverDeletedSecretPath}));
}

View File

@ -62,7 +62,9 @@ int main()
DeleteSecretOperation operation = secretClient.StartDeleteSecret(secret.Name);
// You only need to wait for completion if you want to purge or recover the secret.
operation.PollUntilDone(2s);
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
operation.PollUntilDone(20s);
// purge the deleted secret
secretClient.PurgeDeletedSecret(secret.Name);

View File

@ -74,6 +74,8 @@ int main()
DeleteSecretOperation operation = secretClient.StartDeleteSecret(secret.Name);
// You only need to wait for completion if you want to purge or recover the secret.
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
operation.PollUntilDone(2s);
// purge the deleted secret
secretClient.PurgeDeletedSecret(secret.Name);
@ -85,12 +87,13 @@ int main()
std::cout << "\t-Read from file." << std::endl;
std::ifstream inFile;
inFile.open("backup.dat");
std::vector<uint8_t> inMemoryBackup(backUpSize);
inFile >> inMemoryBackup.data();
BackupSecretResult backedUpSecret;
backedUpSecret.Secret = std::vector<uint8_t>(backUpSize);
inFile >> backedUpSecret.Secret.data();
inFile.close();
std::cout << "\t-Restore Secret" << std::endl;
auto restoredSecret = secretClient.RestoreSecretBackup(inMemoryBackup).Value;
auto restoredSecret = secretClient.RestoreSecretBackup(backedUpSecret).Value;
AssertSecretsEqual(secret, restoredSecret);

View File

@ -84,7 +84,7 @@ Call RestoreSecretBackup to restore a secret from a backup obtained at the pre
```cpp Snippet:SecretSample2RestoreSecret
std::cout << "\t-Restore Secret" << std::endl;
auto restoredSecret = secretClient.RestoreSecretBackup(inMemoryBackup).Value;
auto restoredSecret = secretClient.RestoreSecretBackup(backedUpSecret).Value;
```
## Source

View File

@ -57,6 +57,8 @@ int main()
DeleteSecretOperation operation = secretClient.StartDeleteSecret(secret.Name);
// You only need to wait for completion if you want to purge or recover the secret.
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
operation.PollUntilDone(2s);
// call recover secret
@ -64,13 +66,18 @@ int main()
= secretClient.StartRecoverDeletedSecret(secret.Name);
// poll until done
KeyVaultSecret restoredSecret = recoverOperation.PollUntilDone(2s).Value;
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
SecretProperties restoredSecretProperties = recoverOperation.PollUntilDone(2s).Value;
KeyVaultSecret restoredSecret = secretClient.GetSecret(restoredSecretProperties.Name).Value;
AssertSecretsEqual(secret, restoredSecret);
// cleanup
// start deleting the secret
DeleteSecretOperation cleanupOperation = secretClient.StartDeleteSecret(restoredSecret.Name);
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
cleanupOperation.PollUntilDone(2s);
secretClient.PurgeDeletedSecret(restoredSecret.Name);
}

View File

@ -47,12 +47,14 @@ int main()
KeyVaultSecret secret1 = secretClient.SetSecret(secretName, secretValue).Value;
KeyVaultSecret secret2 = secretClient.SetSecret(secretName2, secretValue).Value;
std::cout << "Secret1 Version : " << secret1.Version() << std::endl;
std::cout << "Secret1 Version : " << secret1.Properties.Version << std::endl;
// get properties of secrets
for (auto secrets = secretClient.GetPropertiesOfSecrets(); secrets.HasPage();
secrets.MoveToNextPage())
{ // go through every secret of each page returned
// the number of results returned for in a page is not guaranteed
// it can be anywhere from 0 to 25
for (auto const& secret : secrets.Items)
{
std::cout << "Found Secret with name: " << secret.Name << std::endl;
@ -64,6 +66,8 @@ int main()
secretsVersion.HasPage();
secretsVersion.MoveToNextPage())
{ // go through each version of the secret
// the number of results returned for in a page is not guaranteed
// it can be anywhere from 0 to 25
for (auto const& secret : secretsVersion.Items)
{
std::cout << "Found Secret with name: " << secret.Name
@ -74,17 +78,23 @@ int main()
// start deleting the secret
DeleteSecretOperation operation = secretClient.StartDeleteSecret(secret1.Name);
// You only need to wait for completion if you want to purge or recover the secret.
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
operation.PollUntilDone(2s);
// start deleting the secret
operation = secretClient.StartDeleteSecret(secret2.Name);
// You only need to wait for completion if you want to purge or recover the secret.
// The duration of the delete operation might vary
// in case returns too fast increase the timeout value
operation.PollUntilDone(2s);
// get all the versions of a secret
for (auto deletedSecrets = secretClient.GetDeletedSecrets(); deletedSecrets.HasPage();
deletedSecrets.MoveToNextPage())
{ // go through each version of the secret
{ // the number of results returned for in a page is not guaranteed
// it can be anywhere from 0 to 25
// go through each deleted secret
for (auto const& deletedSecret : deletedSecrets.Items)
{
std::cout << "Found Secret with name: " << deletedSecret.Name << std::endl;

View File

@ -94,3 +94,14 @@ TEST(DeletedSecretSerializer, GetDeletedClientDeserializeFull3)
Helpers::RunFullExpect(secret, false);
Helpers::RunDeletedExtras(secret);
}
TEST(SecretProperties, FactoryValid)
{
std::string url(
"https://myvault.vault.azure.net/secrets/mysecretname/4387e9f3d6e14c459867679a90fd0f79");
SecretProperties props = SecretProperties::CreateFromURL(url);
EXPECT_EQ(props.Name, "mysecretname");
EXPECT_EQ(props.Version, "4387e9f3d6e14c459867679a90fd0f79");
EXPECT_EQ(props.Id, url);
EXPECT_EQ(props.VaultUrl, "https://myvault.vault.azure.net");
}

View File

@ -151,8 +151,8 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Secrets {
{
EXPECT_EQ(
secret.RecoveryId, "https://myvault.vault.azure.net/deletedsecrets/GetDeletedSecretTest");
EXPECT_EQ(secret.ScheduledPurgeDate.ToString(), "2017-08-02T22:53:53Z");
EXPECT_EQ(secret.DeletedOn.ToString(), "2017-05-04T22:53:53Z");
EXPECT_EQ(secret.ScheduledPurgeDate.Value().ToString(), "2017-08-02T22:53:53Z");
EXPECT_EQ(secret.DeletedOn.Value().ToString(), "2017-05-04T22:53:53Z");
}
};
}}}}} // namespace Azure::Security::KeyVault::Secrets::_test