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

@@ -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.

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.
      *

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();
     };
 

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));

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)

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==";