From bb87bd1f19cd72377f81bdba17ca2583460a6756 Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Tue, 6 Apr 2021 20:02:22 -0700 Subject: [PATCH] Removed `Url::AppendQueryParameters()` since it is no longer used within the SDK. (#2054) * Remove since it is no longer used. * Update usage in test. * Update usage of append qp in tests. * Address PR feedback. --- sdk/core/azure-core/CHANGELOG.md | 2 +- sdk/core/azure-core/inc/azure/core/url.hpp | 16 ++++----- .../test/ut/blob_sas_test.cpp | 16 ++++----- .../test/ut/page_blob_client_test.cpp | 3 +- .../azure-storage-common/test/test_base.cpp | 35 +++++++++++++++++++ .../azure-storage-common/test/test_base.hpp | 4 +++ 6 files changed, 57 insertions(+), 19 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 1165acaf3..0b7a77bf7 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -25,7 +25,7 @@ - Removed `Azure::Core::PackageVersion`. - Removed from `Azure::Core::Http::Policies` namespace: `HttpPolicyOrder`, `TransportPolicy`, `RetryPolicy`, `RequestIdPolicy`, `TelemetryPolicy`, `BearerTokenAuthenticationPolicy`, `LogPolicy`. - Renamed `Azure::Core::Http::RawResponse::GetBodyStream()` to `ExtractBodyStream()`. -- Removed `GetUrlWithoutQuery()` and `GetUrlAuthorityWithScheme()` from `Azure::Core::Url`. +- Removed `AppendQueryParameters()`, `GetUrlWithoutQuery()` and `GetUrlAuthorityWithScheme()` from `Azure::Core::Url`. - Changed the `Azure::Core::Http::HttpMethod` regular enum into an extensible enum class and removed the `HttpMethodToString` helper method. - Removed `Azure::Core::Http::Request::GetHeadersAsString()`. - Introduced `Azure::Core::Context::Key` class which takes place of `std::string` used for `Azure::Core::Context` keys previously. diff --git a/sdk/core/azure-core/inc/azure/core/url.hpp b/sdk/core/azure-core/inc/azure/core/url.hpp index f42e83d98..db3a78ab8 100644 --- a/sdk/core/azure-core/inc/azure/core/url.hpp +++ b/sdk/core/azure-core/inc/azure/core/url.hpp @@ -58,6 +58,14 @@ namespace Azure { namespace Core { std::string GetUrlWithoutQuery(bool relative) const; + /** + * @brief Finds the first '?' symbol and parses everything after it as query parameters. + * separated by '&'. + * + * @param encodedQueryParameters String containing one or more query parameters. + */ + void AppendQueryParameters(const std::string& encodedQueryParameters); + public: /** * @brief Decodes \p value by transforming all escaped characters to it's non-encoded value. @@ -168,14 +176,6 @@ namespace Azure { namespace Core { m_encodedQueryParameters[encodedKey] = encodedValue; } - /** - * @brief Finds the first '?' symbol and parses everything after it as query parameters. - * separated by '&'. - * - * @param encodedQueryParameters String containing one or more query parameters. - */ - void AppendQueryParameters(const std::string& encodedQueryParameters); - /** * @brief Removes an existing query parameter. * diff --git a/sdk/storage/azure-storage-blobs/test/ut/blob_sas_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/blob_sas_test.cpp index 1ce676806..b355510d9 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/blob_sas_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/blob_sas_test.cpp @@ -440,16 +440,16 @@ namespace Azure { namespace Storage { namespace Test { auto verify_blob_snapshot_read = [&](const std::string sas) { Azure::Core::Url blobSnapshotUrlWithSas(blobSnapshotUrl); - blobSnapshotUrlWithSas.AppendQueryParameters(sas); - auto blobSnapshotClient = Blobs::AppendBlobClient(blobSnapshotUrlWithSas.GetAbsoluteUrl()); + auto blobSnapshotClient + = Blobs::AppendBlobClient(AppendQueryParameters(blobSnapshotUrlWithSas, sas)); auto downloadedContent = blobSnapshotClient.Download(); EXPECT_TRUE(ReadBodyStream(downloadedContent.Value.BodyStream).empty()); }; auto verify_blob_snapshot_delete = [&](const std::string sas) { Azure::Core::Url blobSnapshotUrlWithSas(blobSnapshotUrl); - blobSnapshotUrlWithSas.AppendQueryParameters(sas); - auto blobSnapshotClient = Blobs::AppendBlobClient(blobSnapshotUrlWithSas.GetAbsoluteUrl()); + auto blobSnapshotClient + = Blobs::AppendBlobClient(AppendQueryParameters(blobSnapshotUrlWithSas, sas)); EXPECT_NO_THROW(blobSnapshotClient.Delete()); }; @@ -501,16 +501,16 @@ namespace Azure { namespace Storage { namespace Test { auto verify_blob_version_read = [&](const std::string sas) { Azure::Core::Url blobVersionUrlWithSas(blobVersionUrl); - blobVersionUrlWithSas.AppendQueryParameters(sas); - auto blobVersionClient = Blobs::AppendBlobClient(blobVersionUrlWithSas.GetAbsoluteUrl()); + auto blobVersionClient + = Blobs::AppendBlobClient(AppendQueryParameters(blobVersionUrlWithSas, sas)); auto downloadedContent = blobVersionClient.Download(); EXPECT_TRUE(ReadBodyStream(downloadedContent.Value.BodyStream).empty()); }; auto verify_blob_delete_version = [&](const std::string& sas) { Azure::Core::Url blobVersionUrlWithSas(blobVersionUrl); - blobVersionUrlWithSas.AppendQueryParameters(sas); - auto blobVersionClient = Blobs::AppendBlobClient(blobVersionUrlWithSas.GetAbsoluteUrl()); + auto blobVersionClient + = Blobs::AppendBlobClient(AppendQueryParameters(blobVersionUrlWithSas, sas)); blobVersionClient.Delete(); }; diff --git a/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp b/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp index e58fcfc21..13bf34a4d 100644 --- a/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp +++ b/sdk/storage/azure-storage-blobs/test/ut/page_blob_client_test.cpp @@ -147,8 +147,7 @@ namespace Azure { namespace Storage { namespace Test { StandardStorageConnectionString(), m_containerName, RandomString()); std::string snapshot = m_pageBlobClient->CreateSnapshot().Value.Snapshot; Azure::Core::Url sourceUri(m_pageBlobClient->WithSnapshot(snapshot).GetUrl()); - sourceUri.AppendQueryParameters(GetSas()); - auto copyInfo = pageBlobClient.StartCopyIncremental(sourceUri.GetAbsoluteUrl()); + auto copyInfo = pageBlobClient.StartCopyIncremental(AppendQueryParameters(sourceUri, GetSas())); EXPECT_EQ( copyInfo.GetRawResponse().GetStatusCode(), Azure::Core::Http::HttpStatusCode::Accepted); auto getPropertiesResult = copyInfo.PollUntilDone(std::chrono::seconds(1)); diff --git a/sdk/storage/azure-storage-common/test/test_base.cpp b/sdk/storage/azure-storage-common/test/test_base.cpp index 637005e3c..82e7a8717 100644 --- a/sdk/storage/azure-storage-common/test/test_base.cpp +++ b/sdk/storage/azure-storage-common/test/test_base.cpp @@ -141,6 +141,41 @@ namespace Azure { namespace Storage { namespace Test { return connectionString; } + std::string AppendQueryParameters(Azure::Core::Url const& url, std::string const& queryParameters) + { + std::string absoluteUrl = url.GetAbsoluteUrl(); + if (queryParameters.empty()) + { + return absoluteUrl; + } + const auto& existingQP = url.GetQueryParameters(); + bool startWithQuestion = queryParameters[0] == '?'; + if (existingQP.empty()) + { + if (startWithQuestion) + { + absoluteUrl = absoluteUrl + queryParameters; + } + else + { + absoluteUrl = absoluteUrl + '?' + queryParameters; + } + } + else + { + absoluteUrl += '&'; + if (startWithQuestion) + { + absoluteUrl = absoluteUrl + queryParameters.substr(1); + } + else + { + absoluteUrl = absoluteUrl + queryParameters; + } + } + return absoluteUrl; + } + static thread_local std::mt19937_64 random_generator(std::random_device{}()); uint64_t RandomInt(uint64_t minNumber, uint64_t maxNumber) diff --git a/sdk/storage/azure-storage-common/test/test_base.hpp b/sdk/storage/azure-storage-common/test/test_base.hpp index e3502706a..a47f8c137 100644 --- a/sdk/storage/azure-storage-common/test/test_base.hpp +++ b/sdk/storage/azure-storage-common/test/test_base.hpp @@ -43,6 +43,10 @@ namespace Azure { namespace Storage { namespace Test { return x * 1024 * 1024 * 1024 * 1024; } + std::string AppendQueryParameters( + Azure::Core::Url const& url, + std::string const& queryParameters); + const static Azure::ETag DummyETag("0x8D83B58BDF51D75"); const static Azure::ETag DummyETag2("0x8D812645BFB0CDE"); constexpr static const char* DummyMd5 = "tQbD1aMPeB+LiPffUwFQJQ==";