Removed GetUrlWithoutQuery() and GetUrlAuthorityWithScheme() from Azure::Core::Url and added GetScheme(). (#2055)

* Removed GetUrlWithoutQuery() and GetUrlAuthorityWithScheme() from
Azure::Core::Url and added GetScheme().

* Address PR feedback.

sdk/core/azure-core/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## 1.0.0-beta.8 (Unreleased)
 
+### New Features
+
+- Added `Azure::Core::Url::GetScheme()`.
+
 ### Breaking Changes
 
 - Simplified the `Response<T>` API surface to expose two public fields with direct access: `T Value` and a `unique_ptr` to an `Azure::Core::Http::RawResponse`.
@@ -21,6 +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`.
 - Changed the `Azure::Core::Http::HttpMethod` regular enum into an extensible enum class and removed the `HttpMethodToString` helper method.
 - 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

@@ -223,17 +223,9 @@ namespace Azure { namespace Core {
     }
 
     /**
-     * @brief Get Scheme, host, and path, without query parameters.
-     * @return Absolute URL without query parameters.
+     * @brief Get the URL scheme.
      */
-    std::string GetUrlWithoutQuery() const { return GetUrlWithoutQuery(false); }
-
-    /**
-     * @brief Get Scheme, host and port.
-     *
-     * @return Url authority.
-     */
-    std::string GetUrlAuthorityWithScheme() const;
+    const std::string& GetScheme() const { return m_scheme; };
 
     /**
      * @brief Get the path and query parameters.

sdk/core/azure-core/src/http/log_policy.cpp

@@ -37,13 +37,31 @@ inline void AppendHeaders(
   }
 }
 
+inline void LogUrlWithoutQuery(std::ostringstream& log, Url const& url)
+{
+  if (!url.GetScheme().empty())
+  {
+    log << url.GetScheme() << "://";
+  }
+  log << url.GetHost();
+  if (url.GetPort() != 0)
+  {
+    log << ":" << url.GetPort();
+  }
+  if (!url.GetPath().empty())
+  {
+    log << "/" << url.GetPath();
+  }
+}
+
 inline std::string GetRequestLogMessage(LogOptions const& options, Request const& request)
 {
   auto const& requestUrl = request.GetUrl();
 
   std::ostringstream log;
-  log << "HTTP Request : " << request.GetMethod().ToString() << " "
-      << requestUrl.GetUrlWithoutQuery();
+  log << "HTTP Request : " << request.GetMethod().ToString() << " ";
+  LogUrlWithoutQuery(log, requestUrl);
+
   {
     auto encodedRequestQueryParams = requestUrl.GetQueryParameters();
 

sdk/core/azure-core/src/http/url.cpp

@@ -178,29 +178,21 @@ void Url::AppendQueryParameters(const std::string& query)
   }
 }
 
-std::string Url::GetUrlAuthorityWithScheme() const
-{
-  std::string url;
-
-  if (!m_scheme.empty())
-  {
-    url += m_scheme + "://";
-  }
-  url += m_host;
-  if (m_port != 0)
-  {
-    url += ":" + std::to_string(m_port);
-  }
-  return url;
-}
-
 std::string Url::GetUrlWithoutQuery(bool relative) const
 {
   std::string url;
 
   if (!relative)
   {
-    url += GetUrlAuthorityWithScheme();
+    if (!m_scheme.empty())
+    {
+      url += m_scheme + "://";
+    }
+    url += m_host;
+    if (m_port != 0)
+    {
+      url += ":" + std::to_string(m_port);
+    }
   }
 
   if (!m_encodedPath.empty())

sdk/core/azure-core/test/ut/url.cpp

@@ -206,6 +206,28 @@ namespace Azure { namespace Core { namespace Test {
         expected);
   }
 
+  TEST(URL, getScheme)
+  {
+    Core::Url url("http://test.com:9090");
+    std::string expected = "http";
+
+    EXPECT_PRED2(
+        [](std::string expectedValue, std::string value) { return expectedValue == value; },
+        url.GetScheme(),
+        expected);
+  }
+
+  TEST(URL, getSchemeConst)
+  {
+    Core::Url const url("http://test.com:9090");
+    std::string expected = "http";
+
+    EXPECT_PRED2(
+        [](std::string expectedValue, std::string value) { return expectedValue == value; },
+        url.GetScheme(),
+        expected);
+  }
+
   TEST(URL, getPortMax) { EXPECT_THROW(Core::Url url("http://test.com:65540"), std::out_of_range); }
 
   TEST(URL, getPortAfterSet)

sdk/keyvault/azure-security-keyvault-keys/inc/azure/keyvault/keys/details/key_serializers.hpp

@@ -40,11 +40,26 @@ namespace Azure { namespace Security { namespace KeyVault { namespace Keys { nam
         KeyVaultKey& key,
         Azure::Core::Json::_internal::json const& json);
 
+    static std::string GetUrlAuthorityWithScheme(Azure::Core::Url const& url)
+    {
+      std::string urlString;
+      if (!url.GetScheme().empty())
+      {
+        urlString += url.GetScheme() + "://";
+      }
+      urlString += url.GetHost();
+      if (url.GetPort() != 0)
+      {
+        urlString += ":" + std::to_string(url.GetPort());
+      }
+      return urlString;
+    }
+
     void static inline ParseKeyUrl(KeyProperties& keyProperties, std::string const& url)
     {
       Azure::Core::Url kid(url);
       keyProperties.Id = url;
-      keyProperties.VaultUrl = kid.GetUrlAuthorityWithScheme();
+      keyProperties.VaultUrl = GetUrlAuthorityWithScheme(kid);
       auto const& path = kid.GetPath();
       //  path is in the form of `verb/keyName{/keyVersion}`
       auto const separatorChar = '/';