Fix RequestData hashcode and equality for .NET core HttpConnection (#2420)

RequestData's GetHashCode implementation uses instances of types that will return different values per instance (Headers, BasicAuthenticationCredentials).

Remove overridden GetHashCode and Equals methods from RequestData and move hashcode generation into the HttpConnection-CoreFx implementation as it is only used there.

Fixes #2417

Author: russcam

src/Elasticsearch.Net/Configuration/Security/BasicAuthenticationCredentials.cs

@@ -1,6 +1,4 @@
-using System;
-
-namespace Elasticsearch.Net
+namespace Elasticsearch.Net
 {
 	public class BasicAuthenticationCredentials
 	{

src/Elasticsearch.Net/Connection/HttpConnection-CoreFx.cs

@@ -31,30 +31,32 @@ internal class WebProxy : IWebProxy
 	public class HttpConnection : IConnection
 	{
 		private readonly object _lock = new object();
+
 		private readonly ConcurrentDictionary<int, HttpClient> _clients = new ConcurrentDictionary<int, HttpClient>();
 
 		private HttpClient GetClient(RequestData requestData)
 		{
-			var hashCode = requestData.GetHashCode();
+			var key = GetClientKey(requestData);
 			HttpClient client;
-			if (this._clients.TryGetValue(hashCode, out client)) return client;
-			lock (_lock)
+			if (!this._clients.TryGetValue(key, out client))
 			{
-				if (this._clients.TryGetValue(hashCode, out client)) return client;
-
-				var handler = CreateHttpClientHandler(requestData);
-
-				client = new HttpClient(handler, false)
+				lock (_lock)
 				{
-					Timeout = requestData.RequestTimeout
-				};
-
-				client.DefaultRequestHeaders.ExpectContinue = false;
-
-				this._clients.TryAdd(hashCode, client);
-				return client;
+					client = this._clients.GetOrAdd(key, h =>
+					{
+						var handler = CreateHttpClientHandler(requestData);
+						var httpClient = new HttpClient(handler, false)
+						{
+							Timeout = requestData.RequestTimeout
+						};
+
+						httpClient.DefaultRequestHeaders.ExpectContinue = false;
+						return httpClient;
+					});
+				}
 			}
 
+			return client;
 		}
 
 		public virtual ElasticsearchResponse<TReturn> Request<TReturn>(RequestData requestData) where TReturn : class
@@ -189,7 +191,6 @@ protected virtual void SetBasicAuthenticationIfNeeded(HttpRequestMessage request
 			}
 		}
 
-
 		private static System.Net.Http.HttpMethod ConvertHttpMethod(HttpMethod httpMethod)
 		{
 			switch (httpMethod)
@@ -204,6 +205,20 @@ private static System.Net.Http.HttpMethod ConvertHttpMethod(HttpMethod httpMetho
 			}
 		}
 
+		private static int GetClientKey(RequestData requestData)
+		{
+			unchecked
+			{
+				var hashCode = requestData.RequestTimeout.GetHashCode();
+				hashCode = (hashCode * 397) ^ requestData.HttpCompression.GetHashCode();
+				hashCode = (hashCode * 397) ^ (requestData.ProxyAddress?.GetHashCode() ?? 0);
+				hashCode = (hashCode * 397) ^ (requestData.ProxyUsername?.GetHashCode() ?? 0);
+				hashCode = (hashCode * 397) ^ (requestData.ProxyPassword?.GetHashCode() ?? 0);
+				hashCode = (hashCode * 397) ^ requestData.DisableAutomaticProxyDetection.GetHashCode();
+				return hashCode;
+			}
+		}
+
 		void IDisposable.Dispose() => this.DisposeManagedResources();
 
 		protected virtual void DisposeManagedResources()

src/Elasticsearch.Net/Transport/Pipeline/RequestData.cs

@@ -112,55 +112,5 @@ private string CreatePathWithQueryStrings(string path, IConnectionConfigurationV
 				path += "&" + queryString.Substring(1, queryString.Length - 1);
 			return path;
 		}
-
-
-		protected bool Equals(RequestData other) =>
-			RequestTimeout.Equals(other.RequestTimeout)
-			&& PingTimeout.Equals(other.PingTimeout)
-			&& KeepAliveTime == other.KeepAliveTime
-			&& KeepAliveInterval == other.KeepAliveInterval
-			&& Pipelined == other.Pipelined
-			&& HttpCompression == other.HttpCompression
-			&& Equals(Headers, other.Headers)
-			&& string.Equals(RunAs, other.RunAs)
-			&& string.Equals(ProxyAddress, other.ProxyAddress)
-			&& string.Equals(ProxyUsername, other.ProxyUsername)
-			&& string.Equals(ProxyPassword, other.ProxyPassword)
-			&& DisableAutomaticProxyDetection == other.DisableAutomaticProxyDetection
-			&& Equals(BasicAuthorizationCredentials, other.BasicAuthorizationCredentials)
-			&& Equals(ConnectionSettings, other.ConnectionSettings)
-			&& Equals(MemoryStreamFactory, other.MemoryStreamFactory);
-
-		public override bool Equals(object obj)
-		{
-			if (ReferenceEquals(null, obj)) return false;
-			if (ReferenceEquals(this, obj)) return true;
-			if (obj.GetType() != this.GetType()) return false;
-			return Equals((RequestData) obj);
-		}
-
-		public override int GetHashCode()
-		{
-			unchecked
-			{
-				var hashCode = RequestTimeout.GetHashCode();
-				hashCode = (hashCode*397) ^ PingTimeout.GetHashCode();
-				hashCode = (hashCode*397) ^ KeepAliveTime;
-				hashCode = (hashCode*397) ^ KeepAliveInterval;
-				hashCode = (hashCode*397) ^ (RunAs?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ Pipelined.GetHashCode();
-				hashCode = (hashCode*397) ^ HttpCompression.GetHashCode();
-				hashCode = (hashCode*397) ^ (Headers?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ (ProxyAddress?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ (ProxyUsername?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ (ProxyPassword?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ DisableAutomaticProxyDetection.GetHashCode();
-				hashCode = (hashCode*397) ^ (BasicAuthorizationCredentials?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ (ConnectionSettings?.GetHashCode() ?? 0);
-				hashCode = (hashCode*397) ^ (MemoryStreamFactory?.GetHashCode() ?? 0);
-				return hashCode;
-			}
-		}
-
 	}
 }