Use ms for connection timeout (#3001)

* move the static transport adapter to private impl

* use MS for connection timeout

* update

* fix the value

sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp

@@ -14,6 +14,16 @@
 
 namespace Azure { namespace Core { namespace Http {
 
+  namespace _detail {
+    /**
+     * @brief Default maximum time in milliseconds that you allow the connection phase to the server
+     * to take.
+     *
+     */
+    AZ_CORE_DLLEXPORT static const std::chrono::milliseconds DefaultConnectionTimeout
+        = std::chrono::milliseconds(300000);
+  } // namespace _detail
+
   /**
    * @brief The available options to set libcurl SSL options.
    *
@@ -39,13 +49,6 @@ namespace Azure { namespace Core { namespace Http {
    */
   struct CurlTransportOptions final
   {
-    /**
-     * @brief Default Maximum time in seconds that you allow the connection phase to the server to
-     * take.
-     *
-     */
-    AZ_CORE_DLLEXPORT static const long DefaultConnectionTimeout = 300;
-
     /**
      * @brief The string for the proxy is passed directly to the libcurl handle without any parsing
      *
@@ -107,13 +110,14 @@ namespace Azure { namespace Core { namespace Http {
     bool NoSignal = false;
 
     /**
-     * @brief Contain the maximum time in seconds that you allow the connection phase to the server
-     * to take.
+     * @brief Contain the maximum time that you allow the connection phase to the server to take.
      *
      * @details This only limits the connection phase, it has no impact once it has connected.
      *
+     * @remarks The default timeout is 300 seconds and using `0` would set this default value.
+     *
      */
-    long ConnectionTimeout = DefaultConnectionTimeout;
+    std::chrono::milliseconds ConnectionTimeout = _detail::DefaultConnectionTimeout;
   };
 
   /**

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

@@ -1231,8 +1231,12 @@ inline std::string GetConnectionKey(std::string const& host, CurlTransportOption
   key.append(!options.SslOptions.EnableCertificateRevocationListCheck ? "1" : "0");
   key.append(options.SslVerifyPeer ? "1" : "0");
   key.append(options.NoSignal ? "1" : "0");
+  // using DefaultConnectionTimeout or 0 result in the same setting
   key.append(
-      options.ConnectionTimeout == CurlTransportOptions::DefaultConnectionTimeout ? "0" : "1");
+      (options.ConnectionTimeout == Azure::Core::Http::_detail::DefaultConnectionTimeout
+       || options.ConnectionTimeout == std::chrono::milliseconds(0))
+          ? "0"
+          : std::to_string(options.ConnectionTimeout.count()));
 
   return key;
 }
@@ -1338,13 +1342,14 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
         + std::string(curl_easy_strerror(result)));
   }
 
-  if (options.ConnectionTimeout != CurlTransportOptions::DefaultConnectionTimeout)
+  if (options.ConnectionTimeout != Azure::Core::Http::_detail::DefaultConnectionTimeout)
   {
-    if (!SetLibcurlOption(newHandle, CURLOPT_CONNECTTIMEOUT, options.ConnectionTimeout, &result))
+    if (!SetLibcurlOption(newHandle, CURLOPT_CONNECTTIMEOUT_MS, options.ConnectionTimeout, &result))
     {
       throw Azure::Core::Http::TransportException(
           _detail::DefaultFailedToGetNewConnectionTemplate + host
-          + ". Fail setting connect timeout to: " + std::to_string(options.ConnectionTimeout) + ". "
+          + ". Fail setting connect timeout to: "
+          + std::to_string(options.ConnectionTimeout.count()) + " ms. "
           + std::string(curl_easy_strerror(result)));
     }
   }

sdk/core/azure-core/src/http/curl/static_curl.cpp

@@ -301,16 +301,15 @@ public:
       }
     }
 
-    if (m_options.ConnectionTimeout
-        != Azure::Core::Http::CurlTransportOptions::DefaultConnectionTimeout)
+    if (m_options.ConnectionTimeout != Azure::Core::Http::_detail::DefaultConnectionTimeout)
     {
       if (!SetStaticLibcurlOption(
-              m_libcurlHandle, CURLOPT_CONNECTTIMEOUT, m_options.ConnectionTimeout, &result))
+              m_libcurlHandle, CURLOPT_CONNECTTIMEOUT_MS, m_options.ConnectionTimeout, &result))
       {
         throw Azure::Core::Http::TransportException(
-            FailedToGetNewConnectionTemplate + host
-            + ". Fail setting connect timeout to: " + std::to_string(m_options.ConnectionTimeout)
-            + ". " + std::string(curl_easy_strerror(result)));
+            FailedToGetNewConnectionTemplate + host + ". Fail setting connect timeout to: "
+            + std::to_string(m_options.ConnectionTimeout.count()) + ". "
+            + std::string(curl_easy_strerror(result)));
       }
     }
 

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

@@ -114,11 +114,12 @@ namespace Azure { namespace Core { namespace Test {
 
       // Now test that using a different connection config won't re-use the same connection
       std::string const secondExpectedKey
-          = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "001000";
+          = AzureSdkHttpbinServer::Schema() + AzureSdkHttpbinServer::Host() + "00100200000";
       {
         // Creating a new connection with options
         Azure::Core::Http::CurlTransportOptions options;
         options.SslVerifyPeer = false;
+        options.ConnectionTimeout = std::chrono::seconds(200);
         auto connection
             = CurlConnectionPool::g_curlConnectionPool.ExtractOrCreateCurlConnection(req, options);
         EXPECT_EQ(connection->GetConnectionKey(), secondExpectedKey);