Another attempt to fix #2182 #2183 (#2197)

Taking another approach then #2189.

Here we yield responsibility of creating a new client to
ISerializerFactory.

The overloads taking a Func factory are now obsolete and others taking
ISerializerFactory are added

applied feedback on #2197

fix null reference is test setup post switch to serializerfactory

Author: Mpdreamz

src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

@@ -151,7 +151,6 @@ private static void DefaultRequestDataCreated(RequestData response) { }
 		IElasticsearchSerializer IConnectionConfigurationValues.Serializer => _serializer;
 
 		private readonly IConnectionPool _connectionPool;
-		private readonly Func<T, IElasticsearchSerializer> _serializerFactory;
 		IConnectionPool IConnectionConfigurationValues.ConnectionPool => _connectionPool;
 
 		private readonly IConnection _connection;
@@ -164,9 +163,8 @@ protected ConnectionConfiguration(IConnectionPool connectionPool, IConnection co
 		{
 			this._connectionPool = connectionPool;
 			this._connection = connection ?? new HttpConnection();
-			this._serializerFactory = serializerFactory ?? (c=>this.DefaultSerializer((T)this));
 			// ReSharper disable once VirtualMemberCallInContructor
-			this._serializer = _serializerFactory((T)this);
+			this._serializer = serializerFactory?.Invoke((T)this) ?? this.DefaultSerializer((T)this);
 
 			this._requestTimeout = ConnectionConfiguration.DefaultTimeout;
 			this._sniffOnConnectionFault = true;

src/Elasticsearch.Net/Serialization/IElasticsearchSerializer.cs

@@ -26,8 +26,7 @@ public static byte[] SerializeToBytes(this IElasticsearchSerializer serializer,
 				return ms.ToArray();
 			}
 		}
-		public static string SerializeToString(this IElasticsearchSerializer serializer, object data, SerializationFormatting formatting = SerializationFormatting.Indented) => 
+		public static string SerializeToString(this IElasticsearchSerializer serializer, object data, SerializationFormatting formatting = SerializationFormatting.Indented) =>
 			serializer.SerializeToBytes(data, formatting).Utf8String();
 	}
-
-}
+}

src/Nest/CommonAbstractions/ConnectionSettings/ConnectionSettingsBase.cs

@@ -19,16 +19,23 @@ public ConnectionSettings(Uri uri = null)
 			: this(new SingleNodeConnectionPool(uri ?? new Uri("http://localhost:9200"))) { }
 
 		public ConnectionSettings(IConnectionPool connectionPool)
-			: this(connectionPool, null, null) { }
+			: this(connectionPool, null, new SerializerFactory()) { }
 
 		public ConnectionSettings(IConnectionPool connectionPool, IConnection connection)
-			: this(connectionPool, connection, null) { }
+			: this(connectionPool, connection, new SerializerFactory()) { }
 
 		public ConnectionSettings(IConnectionPool connectionPool, Func<ConnectionSettings, IElasticsearchSerializer> serializerFactory)
+#pragma warning disable CS0618 // Type or member is obsolete
 			: this(connectionPool, null, serializerFactory) { }
+#pragma warning restore CS0618 // Type or member is obsolete
 
+		public ConnectionSettings(IConnectionPool connectionPool, IConnection connection, ISerializerFactory serializerFactory)
+			: base(connectionPool, connection, serializerFactory, s => serializerFactory.Create(s)) { }
+
+		[Obsolete("Please use the constructor taking ISerializerFactory instead of a Func")]
 		public ConnectionSettings(IConnectionPool connectionPool, IConnection connection, Func<ConnectionSettings, IElasticsearchSerializer> serializerFactory)
-			: base(connectionPool, connection, serializerFactory) { }
+			: base(connectionPool, connection, null, s => serializerFactory?.Invoke(s)) { }
+
 	}
 
 	/// <summary>
@@ -37,7 +44,7 @@ public ConnectionSettings(IConnectionPool connectionPool, IConnection connection
 	[Browsable(false)]
 	[EditorBrowsable(EditorBrowsableState.Never)]
 	public abstract class ConnectionSettingsBase<TConnectionSettings> : ConnectionConfiguration<TConnectionSettings>, IConnectionSettingsValues
-		where TConnectionSettings : ConnectionSettingsBase<TConnectionSettings>
+		where TConnectionSettings : ConnectionSettingsBase<TConnectionSettings>, IConnectionSettingsValues
 	{
 		private string _defaultIndex;
 		string IConnectionSettingsValues.DefaultIndex => this._defaultIndex;
@@ -63,17 +70,36 @@ public abstract class ConnectionSettingsBase<TConnectionSettings> : ConnectionCo
 		private readonly FluentDictionary<MemberInfo, IPropertyMapping> _propertyMappings = new FluentDictionary<MemberInfo, IPropertyMapping>();
 		FluentDictionary<MemberInfo, IPropertyMapping> IConnectionSettingsValues.PropertyMappings => _propertyMappings;
 
-		protected ConnectionSettingsBase(IConnectionPool connectionPool, IConnection connection, Func<TConnectionSettings, IElasticsearchSerializer> serializerFactory)
-			: base(connectionPool, connection, serializerFactory)
+		private readonly ISerializerFactory _serializerFactory;
+		ISerializerFactory IConnectionSettingsValues.SerializerFactory => _serializerFactory;
+
+		protected ConnectionSettingsBase(
+			IConnectionPool connectionPool,
+			IConnection connection,
+			ISerializerFactory serializerFactory,
+			Func<TConnectionSettings, IElasticsearchSerializer> serializerFactoryFunc
+			)
+			: base(connectionPool, connection, serializerFactoryFunc)
 		{
 			this._defaultTypeNameInferrer = (t => t.Name.ToLowerInvariant());
 			this._defaultFieldNameInferrer = (p => p.ToCamelCase());
 			this._defaultIndices = new FluentDictionary<Type, string>();
 			this._defaultTypeNames = new FluentDictionary<Type, string>();
+			this._serializerFactory = serializerFactory ?? new SerializerFactory();
 
 			this._inferrer = new Inferrer(this);
 		}
 
+		protected ConnectionSettingsBase(
+			IConnectionPool connectionPool,
+			IConnection connection,
+			Func<TConnectionSettings, IElasticsearchSerializer> serializerFactoryFunc
+			)
+			: this(connectionPool, connection, null, serializerFactoryFunc) { }
+
+		IElasticsearchSerializer IConnectionSettingsValues.StatefulSerializer(JsonConverter converter) =>
+			this._serializerFactory.CreateStateful(this, converter);
+
 		/// <summary>
 		/// The default serializer for requests and responses
 		/// </summary>
@@ -92,7 +118,7 @@ public TConnectionSettings PluralizeTypeNames()
 		/// <summary>
 		/// The default index to use when no index is specified.
 		/// </summary>
-		/// <param name="defaultIndex">When null/empty/not set might throw 
+		/// <param name="defaultIndex">When null/empty/not set might throw
 		/// <see cref="NullReferenceException"/> later on when not specifying index explicitly while indexing.
 		/// </param>
 		public TConnectionSettings DefaultIndex(string defaultIndex)
@@ -177,10 +203,10 @@ public TConnectionSettings MapPropertiesFor<TDocument>(Action<PropertyMappingDes
 			var mapper = new PropertyMappingDescriptor<TDocument>();
 			propertiesSelector(mapper);
 			ApplyPropertyMappings(mapper.Mappings);
-			return (TConnectionSettings) this;
+			return (TConnectionSettings)this;
 		}
 
-		private void ApplyPropertyMappings<TDocument>(IList<IClrTypePropertyMapping<TDocument>> mappings) 
+		private void ApplyPropertyMappings<TDocument>(IList<IClrTypePropertyMapping<TDocument>> mappings)
 			where TDocument : class
 		{
 			foreach (var mapping in mappings)
@@ -198,7 +224,7 @@ private void ApplyPropertyMappings<TDocument>(IList<IClrTypePropertyMapping<TDoc
 				{
 					var newName = mapping.NewName;
 					var mappedAs = _propertyMappings[memberInfo].Name;
-					var typeName = typeof (TDocument).Name;
+					var typeName = typeof(TDocument).Name;
 					if (mappedAs.IsNullOrEmpty() && newName.IsNullOrEmpty())
 						throw new ArgumentException("Property mapping '{0}' on type is already ignored"
 							.F(e, newName, mappedAs, typeName));
@@ -228,12 +254,12 @@ public TConnectionSettings InferMappingFor<TDocument>(Func<ClrTypeMappingDescrip
 			if (inferMapping.IdProperty != null)
 #pragma warning disable CS0618 // Type or member is obsolete but will be private in the future OK to call here
 				this.MapIdPropertyFor<TDocument>(inferMapping.IdProperty);
-#pragma warning restore CS0618 
+#pragma warning restore CS0618
 
 			if (inferMapping.Properties != null)
 				this.ApplyPropertyMappings<TDocument>(inferMapping.Properties);
-			
-			return (TConnectionSettings) this;
+
+			return (TConnectionSettings)this;
 		}
 
 	}

src/Nest/CommonAbstractions/ConnectionSettings/IConnectionSettingsValues.cs

@@ -16,5 +16,9 @@ public interface IConnectionSettingsValues : IConnectionConfigurationValues
 		string DefaultIndex { get; }
 		Func<string, string> DefaultFieldNameInferrer { get; }
 		Func<Type, string> DefaultTypeNameInferrer { get; }
+
+		ISerializerFactory SerializerFactory { get; }
+
+		IElasticsearchSerializer StatefulSerializer(JsonConverter converter);
 	}
-}
+}

src/Nest/CommonAbstractions/SerializationBehavior/JsonNetSerializer.cs

@@ -15,27 +15,34 @@ public class JsonNetSerializer : IElasticsearchSerializer
 	{
 		private static readonly Encoding ExpectedEncoding = new UTF8Encoding(false);
 
+
 		protected IConnectionSettingsValues Settings { get; }
 		protected ElasticContractResolver ContractResolver { get; }
 
 		//todo this internal smells
 		internal JsonSerializer Serializer => _defaultSerializer;
 
-		private readonly Dictionary<SerializationFormatting, JsonSerializer> _defaultSerializers;
-		private readonly JsonSerializer _defaultSerializer;
+		private Dictionary<SerializationFormatting, JsonSerializer> _defaultSerializers;
+		private JsonSerializer _defaultSerializer;
 
+		[Obsolete("Use the connection settings constructor that takes an ISerializerFactory")]
 		protected virtual void ModifyJsonSerializerSettings(JsonSerializerSettings settings) { }
+
 		protected virtual IList<Func<Type, JsonConverter>> ContractConverters => null;
 
 		public JsonNetSerializer(IConnectionSettingsValues settings) : this(settings, null) { }
 
 		/// <summary>
 		/// this constructor is only here for stateful (de)serialization
 		/// </summary>
-		internal JsonNetSerializer(IConnectionSettingsValues settings, JsonConverter stateFullConverter)
+		protected internal JsonNetSerializer(
+			IConnectionSettingsValues settings,
+			JsonConverter statefulConverter
+			)
 		{
 			this.Settings = settings;
-			var piggyBackState = stateFullConverter == null ? null : new JsonConverterPiggyBackState { ActualJsonConverter = stateFullConverter };
+
+			var piggyBackState = statefulConverter == null ? null : new JsonConverterPiggyBackState { ActualJsonConverter = statefulConverter };
 			// ReSharper disable once VirtualMemberCallInContructor
 			this.ContractResolver = new ElasticContractResolver(this.Settings, this.ContractConverters) { PiggyBackState = piggyBackState };
 
@@ -48,6 +55,30 @@ internal JsonNetSerializer(IConnectionSettingsValues settings, JsonConverter sta
 			};
 		}
 
+		/// <summary>
+		/// If you subclass JsonNetSerializer and want to apply state passed in the constructor call this to
+		/// overwrite the DefaultSerializers's JsonSerializerSettings and/or connectionsettings.
+		/// </summary>
+		/// <param name="settingsModifier"></param>
+		protected void OverwriteDefaultSerializers(Action<JsonSerializerSettings, IConnectionSettingsValues> settingsModifier)
+		{
+			settingsModifier.ThrowIfNull(nameof(settingsModifier));
+			var collapsed = this.CreateSettings(SerializationFormatting.None);
+			var indented = this.CreateSettings(SerializationFormatting.Indented);
+			settingsModifier(collapsed, this.Settings);
+			settingsModifier(indented, this.Settings);
+
+			this._defaultSerializer = JsonSerializer.Create(collapsed);
+			var indentedSerializer = JsonSerializer.Create(indented);
+			this._defaultSerializers = new Dictionary<SerializationFormatting, JsonSerializer>
+			{
+				{ SerializationFormatting.None, this._defaultSerializer },
+				{ SerializationFormatting.Indented, indentedSerializer }
+			};
+
+		}
+
+
 		public virtual void Serialize(object data, Stream writableStream, SerializationFormatting formatting = SerializationFormatting.Indented)
 		{
 			var serializer = _defaultSerializers[formatting];

src/Nest/CommonAbstractions/SerializationBehavior/SerializerFactory.cs

@@ -0,0 +1,37 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Elasticsearch.Net;
+using Newtonsoft.Json;
+
+namespace Nest
+{
+	public interface ISerializerFactory
+	{
+		IElasticsearchSerializer Create(IConnectionSettingsValues settings);
+
+		IElasticsearchSerializer CreateStateful(IConnectionSettingsValues settings, JsonConverter converter);
+	}
+
+	public class SerializerFactory : ISerializerFactory
+	{
+		private Func<IConnectionSettingsValues, IElasticsearchSerializer> _serializerFactoryFunc;
+
+		public SerializerFactory()
+		{
+
+		}
+		public SerializerFactory(Func<IConnectionSettingsValues , IElasticsearchSerializer> serializerFactoryFunc)
+		{
+			this._serializerFactoryFunc = serializerFactoryFunc;
+		}
+
+		public IElasticsearchSerializer Create(IConnectionSettingsValues settings) =>
+			this._serializerFactoryFunc?.Invoke(settings) ?? new JsonNetSerializer(settings);
+
+		public IElasticsearchSerializer CreateStateful(IConnectionSettingsValues settings, JsonConverter converter) =>
+			new JsonNetSerializer(settings, converter);
+	}
+}

src/Nest/Document/Multiple/MultiGet/ElasticClient-MultiGet.cs

@@ -56,8 +56,8 @@ public IMultiGetResponse MultiGet(IMultiGetRequest request) =>
 				new MultiGetConverter((r, s) => this.DeserializeMultiGetResponse(r, s, CreateCovariantMultiGetConverter(request))),
 				this.LowLevelDispatch.MgetDispatchAsync<MultiGetResponse>
 			);
-		private MultiGetResponse DeserializeMultiGetResponse(IApiCallDetails response, Stream stream, JsonConverter converter)=>
-			new JsonNetSerializer(this.ConnectionSettings, converter).Deserialize<MultiGetResponse>(stream);
+		private MultiGetResponse DeserializeMultiGetResponse(IApiCallDetails response, Stream stream, JsonConverter converter) =>
+			this.ConnectionSettings.StatefulSerializer(converter).Deserialize<MultiGetResponse>(stream);
 
 		private JsonConverter CreateCovariantMultiGetConverter(IMultiGetRequest descriptor) => new MultiGetHitJsonConverter(descriptor);
 

src/Nest/Nest.csproj

@@ -467,6 +467,7 @@
     <Compile Include="CommonAbstractions\SerializationBehavior\GenericJsonConverters\VerbatimDictionaryKeysConverter.cs" />
     <Compile Include="CommonAbstractions\SerializationBehavior\JsonNetSerializer.cs" />
     <Compile Include="CommonAbstractions\SerializationBehavior\JsonReaderExtensions.cs" />
+    <Compile Include="CommonAbstractions\SerializationBehavior\SerializerFactory.cs" />
     <Compile Include="CommonAbstractions\SerializationBehavior\StatefulDeserialization\ConcreteTypeConverter.cs" />
     <Compile Include="CommonAbstractions\SerializationBehavior\StatefulDeserialization\JsonConverterPiggyBackState.cs" />
     <Compile Include="CommonAbstractions\Static\Static.cs" />

src/Nest/Search/MultiSearch/ElasticClient-MultiSearch.cs

@@ -42,7 +42,7 @@ public IMultiSearchResponse MultiSearch(IMultiSearchRequest request)
 				(p, d) =>
 				{
 					var converter = CreateMultiSearchDeserializer(p);
-					var serializer = new JsonNetSerializer(this.ConnectionSettings, converter);
+					var serializer = this.ConnectionSettings.StatefulSerializer(converter);
 					var json = serializer.SerializeToBytes(p).Utf8String();
 					var creator = new MultiSearchCreator((r, s) => serializer.Deserialize<MultiSearchResponse>(s));
 					request.RequestParameters.DeserializationOverride(creator);
@@ -65,7 +65,7 @@ public IMultiSearchResponse MultiSearch(IMultiSearchRequest request)
 				(p, d, c) =>
 				{
 					var converter = CreateMultiSearchDeserializer(p);
-					var serializer = new JsonNetSerializer(this.ConnectionSettings, converter);
+					var serializer = this.ConnectionSettings.StatefulSerializer(converter);
 					var json = serializer.SerializeToBytes(p).Utf8String();
 					var creator = new MultiSearchCreator((r, s) => serializer.Deserialize<MultiSearchResponse>(s));
 					request.RequestParameters.DeserializationOverride(creator);

src/Nest/Search/MultiSearch/MultiSearchResponseJsonConverter.cs

@@ -54,23 +54,23 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
 				var descriptor = m.Descriptor.Value;
 				var concreteTypeSelector = descriptor.TypeSelector;
 				var baseType = m.Descriptor.Value.ClrType ?? typeof(object);
-				
+
 				var generic = MakeDelegateMethodInfo.MakeGenericMethod(baseType);
 
 				if (concreteTypeSelector != null)
 				{
 					var state = typeof(ConcreteTypeConverter<>).CreateGenericInstance(baseType, concreteTypeSelector) as JsonConverter;
 					if (state != null)
 					{
-						var elasticSerializer = new JsonNetSerializer(this._settings, state);
+						var elasticSerializer = this._settings.StatefulSerializer(state);
 
-						generic.Invoke(null, new object[] { m, elasticSerializer.Serializer, response.Responses, this._settings });
+						generic.Invoke(null, new object[] { m, elasticSerializer, response.Responses, this._settings });
 						continue;
 					}
 				}
 				generic.Invoke(null, new object[] { m, serializer, response.Responses, this._settings });
 			}
-			
+
 			return response;
 		}
 
@@ -86,9 +86,9 @@ private class MultiHitTuple
 		}
 
 		private static void CreateMultiHit<T>(
-			MultiHitTuple tuple, 
-			JsonSerializer serializer, 
-			IDictionary<string, object> collection, 
+			MultiHitTuple tuple,
+			JsonSerializer serializer,
+			IDictionary<string, object> collection,
 			IConnectionSettingsValues settings
 		)
 			where T : class