fix #1657 node settings always string, fixes bad sniffs. Added integration tests for sniffing and fixed an issue with how global query strings are appended (e.g ?pretty=true)

Author: Mpdreamz

src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

@@ -3,7 +3,7 @@
 using System.ComponentModel;
 
 namespace Elasticsearch.Net
-{
+{
 	/// <summary>
 	/// ConnectionConfiguration allows you to control how ElasticsearchClient behaves and where/how it connects 
 	/// to elasticsearch
@@ -12,36 +12,41 @@ public class ConnectionConfiguration : ConnectionConfiguration<ConnectionConfigu
 	{
 		public static TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);
 		public static TimeSpan DefaultPingTimeout = TimeSpan.FromSeconds(2);
-		public static TimeSpan DefaultPingTimeoutOnSSL = TimeSpan.FromSeconds(5);
-
+		public static TimeSpan DefaultPingTimeoutOnSSL = TimeSpan.FromSeconds(5);
+
 		/// <summary>
 		/// ConnectionConfiguration allows you to control how ElasticsearchClient behaves and where/how it connects 
 		/// to elasticsearch
 		/// </summary>
 		/// <param name="uri">The root of the elasticsearch node we want to connect to. Defaults to http://localhost:9200</param>
 		public ConnectionConfiguration(Uri uri = null)
-			: this(new SingleNodeConnectionPool(uri ?? new Uri("http://localhost:9200"))) { }
-
+			: this(new SingleNodeConnectionPool(uri ?? new Uri("http://localhost:9200")))
+		{ }
+
 		/// <summary>
 		/// ConnectionConfiguration allows you to control how ElasticsearchClient behaves and where/how it connects 
 		/// to elasticsearch
 		/// </summary>
 		/// <param name="connectionPool">A connection pool implementation that'll tell the client what nodes are available</param>
-		public ConnectionConfiguration(IConnectionPool connectionPool)
-			// ReSharper disable once IntroduceOptionalParameters.Global 
-			: this(connectionPool, null, null) { }
+		public ConnectionConfiguration(IConnectionPool connectionPool)
+			// ReSharper disable once IntroduceOptionalParameters.Global 
+			: this(connectionPool, null, null)
+		{ }
 
-		public ConnectionConfiguration(IConnectionPool connectionPool, IConnection connection)
-			// ReSharper disable once IntroduceOptionalParameters.Global
-			: this(connectionPool, connection, null) { }
+		public ConnectionConfiguration(IConnectionPool connectionPool, IConnection connection)
+			// ReSharper disable once IntroduceOptionalParameters.Global
+			: this(connectionPool, connection, null)
+		{ }
 
 		public ConnectionConfiguration(IConnectionPool connectionPool, IElasticsearchSerializer serializer)
-			: this(connectionPool, null, serializer) { }
-
-		// ReSharper disable once MemberCanBePrivate.Global
-		// eventhough we use don't use this we very much would like to  expose this constructor
+			: this(connectionPool, null, serializer)
+		{ }
+
+		// ReSharper disable once MemberCanBePrivate.Global
+		// eventhough we use don't use this we very much would like to  expose this constructor
 		public ConnectionConfiguration(IConnectionPool connectionPool, IConnection connection, IElasticsearchSerializer serializer)
-			: base(connectionPool, connection, serializer) { }
+			: base(connectionPool, connection, serializer)
+		{ }
 	}
 
 	[Browsable(false)]
@@ -73,7 +78,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
 		private string _proxyUsername;
 		string IConnectionConfigurationValues.ProxyUsername => _proxyUsername;
 
-		private  string _proxyPassword;
+		private string _proxyPassword;
 		string IConnectionConfigurationValues.ProxyPassword => _proxyPassword;
 
 		private bool _disablePings;
@@ -112,7 +117,7 @@ public abstract class ConnectionConfiguration<T> : IConnectionConfigurationValue
 		private bool _throwExceptions;
 		bool IConnectionConfigurationValues.ThrowExceptions => _throwExceptions;
 
-		private static void DefaultApiCallHandler(IApiCallDetails status) {}
+		private static void DefaultApiCallHandler(IApiCallDetails status) { }
 		Action<IApiCallDetails> _apiCallHandler = DefaultApiCallHandler;
 		Action<IApiCallDetails> IConnectionConfigurationValues.ApiCallHandler => _apiCallHandler;
 
@@ -135,13 +140,13 @@ private static void DefaultApiCallHandler(IApiCallDetails status) {}
 		IConnection IConnectionConfigurationValues.Connection => _connection;
 
 		[System.Diagnostics.CodeAnalysis.SuppressMessage(
-			"Potential Code Quality Issues", "RECS0021:Warns about calls to virtual member functions occuring in the constructor", 
+			"Potential Code Quality Issues", "RECS0021:Warns about calls to virtual member functions occuring in the constructor",
 			Justification = "We want the virtual method to run on most derived")]
 		protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection connection, IElasticsearchSerializer serializer)
 		{
 			this._connectionPool = connectionPool;
-			this._connection = connection ?? new HttpConnection();
-			// ReSharper disable once VirtualMemberCallInContructor
+			this._connection = connection ?? new HttpConnection();
+			// ReSharper disable once VirtualMemberCallInContructor
 			this._serializer = serializer ?? this.DefaultSerializer();
 
 			this._requestTimeout = ConnectionConfiguration.DefaultTimeout;
@@ -157,95 +162,95 @@ protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection co
 		public T EnableTcpKeepAlive(TimeSpan keepAliveTime, TimeSpan keepAliveInterval) =>
 			Assign(a => { this._keepAliveTime = keepAliveTime; this._keepAliveInterval = keepAliveInterval; });
 
-		public T MaximumRetries(int maxRetries) => Assign(a => a._maxRetries = maxRetries);
-
+		public T MaximumRetries(int maxRetries) => Assign(a => a._maxRetries = maxRetries);
+
 		/// <summary>
 		/// On connection pools that support reseeding setting this to true (default) will resniff the cluster when a call fails
 		/// </summary>
-		public T SniffOnConnectionFault(bool sniffsOnConnectionFault = true) => Assign(a => a._sniffOnConnectionFault = sniffsOnConnectionFault);
-
+		public T SniffOnConnectionFault(bool sniffsOnConnectionFault = true) => Assign(a => a._sniffOnConnectionFault = sniffsOnConnectionFault);
+
 		/// <summary>
 		/// Enables sniffing on first usage of a connection pool if that pool supports reseeding, defaults to true
 		/// </summary>
-		public T SniffOnStartup(bool sniffsOnStartup = true) => Assign(a => a._sniffOnStartup = sniffsOnStartup);
-
+		public T SniffOnStartup(bool sniffsOnStartup = true) => Assign(a => a._sniffOnStartup = sniffsOnStartup);
+
 		/// <summary>
 		/// Set the duration after which a cluster state is considered stale and a sniff should be performed again.
 		/// An IConnectionPool has to signal it supports reseeding otherwise sniffing will never happen.
 		/// Defaults to 1 hour.
 		/// Set to null to disable completely. Sniffing will only ever happen on ConnectionPools that return true for SupportsReseeding
 		/// </summary>
 		/// <param name="sniffLifeSpan">The duration a clusterstate is considered fresh, set to null to disable periodic sniffing</param>
-		public T SniffLifeSpan(TimeSpan? sniffLifeSpan) => Assign(a => a._sniffLifeSpan = sniffLifeSpan);
-
+		public T SniffLifeSpan(TimeSpan? sniffLifeSpan) => Assign(a => a._sniffLifeSpan = sniffLifeSpan);
+
 		/// <summary>
 		/// Enable gzip compressed requests and responses, do note that you need to configure elasticsearch to set this
 		/// <para>http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/modules-http.html"</para>
 		/// </summary>
 		public T EnableHttpCompression(bool enabled = true) => Assign(a => a._enableHttpCompression = enabled);
 
-		public T DisableAutomaticProxyDetection(bool disable = true) => Assign(a => a._disableAutomaticProxyDetection = disable);
-
+		public T DisableAutomaticProxyDetection(bool disable = true) => Assign(a => a._disableAutomaticProxyDetection = disable);
+
 		/// <summary>
 		/// Instead of following a c/go like error checking on response.IsValid always throw an exception
 		/// on the client when a call resulted in an exception on either the client or the Elasticsearch server. 
 		/// <para>Reasons for such exceptions could be search parser errors, index missing exceptions, etc...</para>
 		/// </summary>
-		public T ThrowExceptions(bool alwaysThrow = true) => Assign(a => a._throwExceptions = alwaysThrow);
-
+		public T ThrowExceptions(bool alwaysThrow = true) => Assign(a => a._throwExceptions = alwaysThrow);
+
 		/// <summary>
 		/// When a node is used for the very first time or when it's used for the first time after it has been marked dead
 		/// a ping with a very low timeout is send to the node to make sure that when it's still dead it reports it as fast as possible.
 		/// You can disable these pings globally here if you rather have it fail on the possible slower original request
 		/// </summary>
-		public T DisablePing(bool disable = true) => Assign(a => a._disablePings = disable);
-
+		public T DisablePing(bool disable = true) => Assign(a => a._disablePings = disable);
+
 		/// <summary>
 		/// This NameValueCollection will be appended to every url NEST calls, great if you need to pass i.e an API key.
 		/// </summary>
-		public T GlobalQueryStringParameters(NameValueCollection queryStringParameters) => Assign(a => a._queryString.Add(queryStringParameters));
-
+		public T GlobalQueryStringParameters(NameValueCollection queryStringParameters) => Assign(a => a._queryString.Add(queryStringParameters));
+
 		/// <summary>
 		/// a NameValueCollection that will be send as headers for each request
 		/// </summary>
-		public T GlobalHeaders(NameValueCollection headers) => Assign(a => a._headers.Add(headers));
-
+		public T GlobalHeaders(NameValueCollection headers) => Assign(a => a._headers.Add(headers));
+
 		/// <summary>
 		/// Sets the default timeout in milliseconds for each request to Elasticsearch.
 		/// NOTE: You can set this to a high value here, and specify the timeout on Elasticsearch's side.
 		/// </summary>
 		/// <param name="timeout">time out in milliseconds</param>
-		public T RequestTimeout(TimeSpan timeout) => Assign(a => a._requestTimeout = timeout);
-
+		public T RequestTimeout(TimeSpan timeout) => Assign(a => a._requestTimeout = timeout);
+
 		/// <summary>
 		/// Sets the default ping timeout in milliseconds for ping requests, which are used
 		/// to determine whether a node is alive. Pings should fail as fast as possible.
 		/// </summary>
 		/// <param name="timeout">The ping timeout in milliseconds defaults to 1000, or 2000 is using SSL.</param>
-		public T PingTimeout(TimeSpan timeout) => Assign(a => a._pingTimeout = timeout);
-
+		public T PingTimeout(TimeSpan timeout) => Assign(a => a._pingTimeout = timeout);
+
 		/// <summary>
 		/// Sets the default dead timeout factor when a node has been marked dead.
 		/// </summary>
 		/// <remarks>Some connection pools may use a flat timeout whilst others take this factor and increase it exponentially</remarks>
 		/// <param name="timeout"></param>
-		public T DeadTimeout(TimeSpan timeout) => Assign(a => a._deadTimeout = timeout);
-
+		public T DeadTimeout(TimeSpan timeout) => Assign(a => a._deadTimeout = timeout);
+
 		/// <summary>
 		/// Sets the maximum time a node can be marked dead. 
 		/// Different implementations of IConnectionPool may choose a different default.
 		/// </summary>
 		/// <param name="timeout">The timeout in milliseconds</param>
-		public T MaxDeadTimeout(TimeSpan timeout) => Assign(a => a._maxDeadTimeout = timeout);
-
+		public T MaxDeadTimeout(TimeSpan timeout) => Assign(a => a._maxDeadTimeout = timeout);
+
 		/// <summary>
 		/// Limits the total runtime including retries separately from <see cref="RequestTimeout"/>
 		/// <pre>
 		/// When not specified defaults to <see cref="RequestTimeout"/> which itself defaults to 60seconds
 		/// </pre>
 		/// </summary>
-		public T MaxRetryTimeout(TimeSpan maxRetryTimeout) => Assign(a => a._maxRetryTimeout = maxRetryTimeout);
-
+		public T MaxRetryTimeout(TimeSpan maxRetryTimeout) => Assign(a => a._maxRetryTimeout = maxRetryTimeout);
+
 		/// <summary>
 		/// If your connection has to go through proxy use this method to specify the proxy url
 		/// </summary>
@@ -256,32 +261,33 @@ public T Proxy(Uri proxyAdress, string username, string password)
 			this._proxyUsername = username;
 			this._proxyPassword = password;
 			return (T)this;
-		}
-
+		}
+
 		/// <summary>
 		/// Forces all requests to have ?pretty=true, causing elasticsearch to return formatted json. 
 		/// Also forces the client to send out formatted json. Defaults to false
 		/// </summary>
-		public T PrettyJson(bool b = true) => Assign(a => {
+		public T PrettyJson(bool b = true) => Assign(a =>
+		{
 			this._prettyJson = b;
 			if (!b && this._queryString["pretty"] != null) this._queryString.Remove("pretty");
 			else if (b && this._queryString["pretty"] == null)
-				this.GlobalQueryStringParameters(new NameValueCollection { { "pretty", b.ToString().ToLowerInvariant() } });
-        });
-
+				this.GlobalQueryStringParameters(new NameValueCollection { { "pretty", b.ToString().ToLowerInvariant() } });
+		});
+
 		/// <summary>
 		/// Make sure the reponse bytes are always available on the ElasticsearchResponse object
 		/// <para>Note: that depending on the registered serializer this may cause the respond to be read in memory first</para>
 		/// </summary>
-		public T DisableDirectStreaming(bool b = true) => Assign(a => a._disableDirectStreaming = b);
-
+		public T DisableDirectStreaming(bool b = true) => Assign(a => a._disableDirectStreaming = b);
+
 		/// <summary>
 		/// Global callback for every response that NEST receives, useful for custom logging.
 		/// Calling this multiple times will register multiple listeners
 		/// </summary>
 		public T ConnectionStatusHandler(Action<IApiCallDetails> handler) =>
-			Assign(a => a._apiCallHandler += handler ?? DefaultApiCallHandler);
-
+			Assign(a => a._apiCallHandler += handler ?? DefaultApiCallHandler);
+
 		/// <summary>
 		/// Basic access authentication credentials to specify with all requests.
 		/// </summary>
@@ -293,8 +299,8 @@ public T BasicAuthentication(string userName, string password)
 				Password = password
 			};
 			return (T)this;
-		}
-
+		}
+
 		/// <summary>
 		/// Allows for requests to be pipelined. http://en.wikipedia.org/wiki/HTTP_pipelining
 		/// <para>Note: HTTP pipelining must also be enabled in Elasticsearch for this to work properly.</para>

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

@@ -84,6 +84,7 @@ public RequestData(HttpMethod method, string path, PostData<object> data, IConne
 
 		private string CreatePathWithQueryStrings(string path, IConnectionConfigurationValues global, IRequestParameters request = null)
 		{
+
 			//Make sure we append global query string as well the request specific query string parameters
 			var copy = new NameValueCollection(global.QueryStringParameters);
 			var formatter = new UrlFormatProvider(this.ConnectionSettings);
@@ -92,7 +93,11 @@ private string CreatePathWithQueryStrings(string path, IConnectionConfigurationV
 			if (!copy.HasKeys()) return path;
 
 			var queryString = copy.ToQueryString();
-			path += queryString;
+			var tempUri = new Uri("http://localhost:9200/" + path).Purify();
+			if (tempUri.Query.IsNullOrEmpty())
+				path += queryString;
+			else 
+				path += "&" + queryString.Substring(1, queryString.Length - 1);
 			return path;
 		}
 	}

src/Elasticsearch.Net/Transport/Sniff/SniffResponse.cs

@@ -32,10 +32,10 @@ internal class SniffNode
 		public string ip { get; set; }
 		public string version { get; set; }
 		public string build { get; set; }
-		public IDictionary<string, object> settings { get; set; }
+		public IDictionary<string, string> settings { get; set; }
 
-		internal bool MasterEligable => !((this.settings?.ContainsKey("node.master")).GetValueOrDefault(false) && ((bool)this.settings["node.master"]) == false);
-		internal bool HoldsData => !((this.settings?.ContainsKey("node.data")).GetValueOrDefault(false) && ((bool)this.settings["node.data"]) == false);
+		internal bool MasterEligable => !((this.settings?.ContainsKey("node.master")).GetValueOrDefault(false) && Convert.ToBoolean(this.settings["node.master"]) == false);
+		internal bool HoldsData => !((this.settings?.ContainsKey("node.data")).GetValueOrDefault(false) && Convert.ToBoolean(this.settings["node.data"]) == false);
 	}
 
 }

src/Nest/IElasticClient.cs

@@ -4,6 +4,7 @@ namespace Nest
 {
 	public partial interface IElasticClient
 	{
+		IConnectionSettingsValues ConnectionSettings { get; }
 		IElasticsearchSerializer Serializer { get; }
 		IElasticsearchClient Raw { get; }
 		ElasticInferrer Infer { get; }