From 608ba2bb5fffbc2b6950fdb3d69f137e1bd80da5 Mon Sep 17 00:00:00 2001
From: Dor Blayzer <59066376+Dor-bl@users.noreply.github.com>
Date: Fri, 12 Jan 2024 10:22:31 +0200
Subject: [PATCH] refactor: Replace obsolete WebRequest,HttpWebRequest with
 HttpClient (#720)

* refactor: Replace obsolete WebRequest,HttpWebRequest with HttpClient

* chore: Reuse the same HttpClient instance across multiple requests.

* chore: Review fixes

* chore: Move ConfigureAwait(false) to the calling method

* fix: Revert disposing of SharedHttpClient

* chore: add ConfigureAwait(false) to GetResponse

* refactor: Extract method for shared HttpClient initialization

* chore: Introduce StartAsync to remove the usage of .Result

* chore: Introduce IsRunningAsync method in order to remove the usage of .Result from IsRunning method

* chore: add ConfigureAwait(false) to PingAsync calls

* chore: make SharedHttpClient a field
---
 .../Appium/Service/AppiumLocalService.cs      | 73 ++++++++++++++-----
 1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/src/Appium.Net/Appium/Service/AppiumLocalService.cs b/src/Appium.Net/Appium/Service/AppiumLocalService.cs
index b66ef2cf..d643a147 100644
--- a/src/Appium.Net/Appium/Service/AppiumLocalService.cs
+++ b/src/Appium.Net/Appium/Service/AppiumLocalService.cs
@@ -20,10 +20,15 @@
 using System.IO;
 using System.Linq;
 using System.Net;
+using System.Net.Http;
 using System.Runtime.CompilerServices;
+using System.Threading.Tasks;
 
 namespace OpenQA.Selenium.Appium.Service
 {
+    /// <summary>
+    /// Represents a local Appium server service that can be started and stopped programmatically.
+    /// </summary>
     public class AppiumLocalService : ICommandServer
     {
         private readonly FileInfo NodeJS;
@@ -32,6 +37,7 @@ public class AppiumLocalService : ICommandServer
         private readonly int Port;
         private readonly TimeSpan InitializationTimeout;
         private readonly IDictionary<string, string> EnvironmentForProcess;
+        private readonly HttpClient SharedHttpClient;
         private Process Service;
         private List<string> ArgsList;
 
@@ -55,6 +61,19 @@ internal AppiumLocalService(
             Port = port;
             InitializationTimeout = initializationTimeout;
             EnvironmentForProcess = environmentForProcess;
+            SharedHttpClient = CreateHttpClientInstance;
+        }
+
+        private HttpClient CreateHttpClientInstance
+        {
+            get
+            {
+                SocketsHttpHandler handler = new SocketsHttpHandler
+                {
+                    PooledConnectionLifetime = TimeSpan.FromMinutes(2)
+                };
+                return new HttpClient(handler);
+            }
         }
 
         /// <summary>
@@ -68,10 +87,20 @@ internal AppiumLocalService(
         public event DataReceivedEventHandler OutputDataReceived;
 
         /// <summary>
-        /// Starts the defined appium server
+        /// Starts the defined Appium server.
+        /// <remarks>
+        /// <para>
+        /// This method executes the synchronous version of starting the Appium server.
+        /// </para>
+        /// </remarks>
         /// </summary>
         [MethodImpl(MethodImplOptions.Synchronized)]
         public void Start()
+        {
+            StartAsync().GetAwaiter().GetResult();
+        }
+
+        private async Task StartAsync()
         {
             if (IsRunning)
             {
@@ -112,7 +141,7 @@ public void Start()
                 throw new AppiumServerHasNotBeenStartedLocallyException(msgTxt, e);
             }
 
-            isLaunched = Ping(InitializationTimeout);
+            isLaunched = await PingAsync(InitializationTimeout).ConfigureAwait(false);
             if (!isLaunched)
             {
                 DestroyProcess();
@@ -138,7 +167,9 @@ private void DestroyProcess()
             }
             finally
             {
-                Service.Close();
+                Service?.Close();
+
+                SharedHttpClient.Dispose();
             }
         }
 
@@ -173,14 +204,19 @@ public bool IsRunning
                     return false;
                 }
 
-                return Ping(new TimeSpan(0, 0, 0, 0, 500));
+                return IsRunningAsync(TimeSpan.FromMilliseconds(500)).GetAwaiter().GetResult();
             }
         }
 
+        private async Task<bool> IsRunningAsync(TimeSpan timeout)
+        {
+            return await PingAsync(timeout).ConfigureAwait(false);
+        }
+
         private string GetArgsValue(string argStr)
         {
             int idx;
-            idx= ArgsList.IndexOf(argStr);
+            idx = ArgsList.IndexOf(argStr);
             return ArgsList[idx + 1];
         }
 
@@ -199,7 +235,7 @@ private string ParseBasePath()
 
         private void GenerateArgsList()
         {
-           ArgsList = Arguments.Split(' ').ToList();
+            ArgsList = Arguments.Split(' ').ToList();
         }
         private Uri CreateStatusUrl()
         {
@@ -215,7 +251,7 @@ private Uri CreateStatusUrl()
 
             if (service.IsLoopback || IP.ToString().Equals(AppiumServiceConstants.DefaultLocalIPAddress))
             {
-                var tmpStatus = "http://localhost:" + Convert.ToString(Port);
+                string tmpStatus = "http://localhost:" + Convert.ToString(Port);
                 if (defBasePath)
                 {
                     status = new Uri(tmpStatus + AppiumServiceConstants.StatusUrl);
@@ -239,7 +275,7 @@ private Uri CreateStatusUrl()
             return status;
         }
 
-        private bool Ping(TimeSpan span)
+        private async Task<bool> PingAsync(TimeSpan span)
         {
             bool pinged = false;
 
@@ -250,28 +286,27 @@ private bool Ping(TimeSpan span)
             DateTime endTime = DateTime.Now.Add(span);
             while (!pinged & DateTime.Now < endTime)
             {
-                HttpWebRequest request = (HttpWebRequest)WebRequest.Create(status);
-                HttpWebResponse response = null;
                 try
                 {
-                    using (response = (HttpWebResponse) request.GetResponse())
+                    HttpResponseMessage response = await GetHttpResponseAsync(status).ConfigureAwait(false);
+
+                    if (response.IsSuccessStatusCode)
                     {
-                        pinged = true;
+                        return true;
                     }
                 }
                 catch
                 {
                     pinged = false;
                 }
-                finally
-                {
-                    if (response != null)
-                    {
-                        response.Close();
-                    }
-                }
             }
             return pinged;
         }
+
+        private async Task<HttpResponseMessage> GetHttpResponseAsync(Uri status)
+        {
+            HttpResponseMessage response = await SharedHttpClient.GetAsync(status).ConfigureAwait(false);
+            return response;
+        }
     }
 }
\ No newline at end of file