Fixes for custom serialization settings

Previously, ModifyJsonSerializationSettings() did not respect properties
initialized in a custom serializer's constructor, since it was being
called in the base class constructor. This is bad practice (hence the
ReSharper warning).  This change moves all of the initialization code
that was in the ctor to a new method Initialize() which must be called
after the serializer is instantiated.

This change also fixes another issue where our stateful converters were
totally disregarding any registered custom serializer.  For this reason,
Initialize() also accepts an optional JsonConverter which is used to
return a new serializer (custom or default) with the stateful converter
applied.

Relates #2183
Relates #2182

Author: gmarz

src/Nest/CommonAbstractions/ConnectionSettings/ConnectionSettingsBase.cs

@@ -71,14 +71,23 @@ protected ConnectionSettingsBase(IConnectionPool connectionPool, IConnection con
 			this._defaultIndices = new FluentDictionary<Type, string>();
 			this._defaultTypeNames = new FluentDictionary<Type, string>();
 
+			var serializer = ((IConnectionConfigurationValues)this).Serializer as JsonNetSerializer;
+			if (serializer != null)
+				serializer.Initialize();
+
 			this._inferrer = new Inferrer(this);
 		}
 
 		/// <summary>
 		/// The default serializer for requests and responses
 		/// </summary>
 		/// <returns></returns>
-		protected override IElasticsearchSerializer DefaultSerializer(TConnectionSettings settings) => new JsonNetSerializer(settings);
+		protected override IElasticsearchSerializer DefaultSerializer(TConnectionSettings settings)
+		{
+			var serializer = new JsonNetSerializer(settings);
+			serializer.Initialize();
+			return serializer;
+		}
 
 		/// <summary>
 		/// This calls SetDefaultTypenameInferrer with an implementation that will pluralize type names. This used to be the default prior to Nest 0.90
@@ -92,7 +101,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)
@@ -180,7 +189,7 @@ public TConnectionSettings MapPropertiesFor<TDocument>(Action<PropertyMappingDes
 			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)
@@ -228,11 +237,11 @@ 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;
 		}
 

src/Nest/CommonAbstractions/SerializationBehavior/JsonNetSerializer.cs

@@ -16,27 +16,25 @@ public class JsonNetSerializer : IElasticsearchSerializer
 		private static readonly Encoding ExpectedEncoding = new UTF8Encoding(false);
 
 		protected IConnectionSettingsValues Settings { get; }
-		protected ElasticContractResolver ContractResolver { get; }
+		protected ElasticContractResolver ContractResolver { get; private set; }
 
 		//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;
 
 		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)
+		public JsonNetSerializer(IConnectionSettingsValues settings)
 		{
 			this.Settings = settings;
-			var piggyBackState = stateFullConverter == null ? null : new JsonConverterPiggyBackState { ActualJsonConverter = stateFullConverter };
-			// ReSharper disable once VirtualMemberCallInContructor
+		}
+
+		public void Initialize(JsonConverter statefulConverter = null)
+		{
+			var piggyBackState = statefulConverter == null ? null : new JsonConverterPiggyBackState { ActualJsonConverter = statefulConverter };
 			this.ContractResolver = new ElasticContractResolver(this.Settings, this.ContractConverters) { PiggyBackState = piggyBackState };
 
 			this._defaultSerializer = JsonSerializer.Create(this.CreateSettings(SerializationFormatting.None));

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

@@ -1,12 +1,12 @@
 using System;
 using System.IO;
+using System.Threading;
 using System.Threading.Tasks;
 using Elasticsearch.Net;
 using Newtonsoft.Json;
 
 namespace Nest
 {
-	using System.Threading;
 	using MultiGetConverter = Func<IApiCallDetails, Stream, MultiGetResponse>;
 
 	public partial interface IElasticClient
@@ -40,7 +40,7 @@ public IMultiGetResponse MultiGet(Func<MultiGetDescriptor, IMultiGetRequest> sel
 		public IMultiGetResponse MultiGet(IMultiGetRequest request) =>
 			this.Dispatcher.Dispatch<IMultiGetRequest, MultiGetRequestParameters, MultiGetResponse>(
 				request,
-				new MultiGetConverter((r, s) => this.DeserializeMultiGetResponse(r, s, CreateCovariantMultiGetConverter(request))),
+				new MultiGetConverter((r, s) => this.DeserializeMultiGetResponse(r, s, new MultiGetHitJsonConverter(request))),
 				this.LowLevelDispatch.MgetDispatch<MultiGetResponse>
 			);
 
@@ -53,13 +53,15 @@ public IMultiGetResponse MultiGet(IMultiGetRequest request) =>
 			this.Dispatcher.DispatchAsync<IMultiGetRequest, MultiGetRequestParameters, MultiGetResponse, IMultiGetResponse>(
 				request,
 				cancellationToken,
-				new MultiGetConverter((r, s) => this.DeserializeMultiGetResponse(r, s, CreateCovariantMultiGetConverter(request))),
+				new MultiGetConverter((r, s) => this.DeserializeMultiGetResponse(r, s, new MultiGetHitJsonConverter(request))),
 				this.LowLevelDispatch.MgetDispatchAsync<MultiGetResponse>
 			);
-		private MultiGetResponse DeserializeMultiGetResponse(IApiCallDetails response, Stream stream, JsonConverter converter)=>
-			new JsonNetSerializer(this.ConnectionSettings, converter).Deserialize<MultiGetResponse>(stream);
-
-		private JsonConverter CreateCovariantMultiGetConverter(IMultiGetRequest descriptor) => new MultiGetHitJsonConverter(descriptor);
 
+		private MultiGetResponse DeserializeMultiGetResponse(IApiCallDetails response, Stream stream, JsonConverter converter)
+		{
+			var serializer = this.Serializer as JsonNetSerializer ?? new JsonNetSerializer(this.ConnectionSettings);
+			serializer.Initialize(converter);
+			return serializer.Deserialize<MultiGetResponse>(stream);
+		}
 	}
 }

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

@@ -42,7 +42,8 @@ public IMultiSearchResponse MultiSearch(IMultiSearchRequest request)
 				(p, d) =>
 				{
 					var converter = CreateMultiSearchDeserializer(p);
-					var serializer = new JsonNetSerializer(this.ConnectionSettings, converter);
+					var serializer = this.Serializer as JsonNetSerializer ?? new JsonNetSerializer(this.ConnectionSettings);
+					serializer.Initialize(converter);
 					var json = serializer.SerializeToBytes(p).Utf8String();
 					var creator = new MultiSearchCreator((r, s) => serializer.Deserialize<MultiSearchResponse>(s));
 					request.RequestParameters.DeserializationOverride(creator);
@@ -65,7 +66,8 @@ public IMultiSearchResponse MultiSearch(IMultiSearchRequest request)
 				(p, d, c) =>
 				{
 					var converter = CreateMultiSearchDeserializer(p);
-					var serializer = new JsonNetSerializer(this.ConnectionSettings, converter);
+					var serializer = this.Serializer as JsonNetSerializer ?? new JsonNetSerializer(this.ConnectionSettings);
+					serializer.Initialize(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,24 @@ 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 = new JsonNetSerializer(this._settings);
+						elasticSerializer.Initialize(state);
 
 						generic.Invoke(null, new object[] { m, elasticSerializer.Serializer, response.Responses, this._settings });
 						continue;
 					}
 				}
 				generic.Invoke(null, new object[] { m, serializer, response.Responses, this._settings });
 			}
-			
+
 			return response;
 		}
 
@@ -86,9 +87,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

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

@@ -10,7 +10,7 @@ public partial class ElasticClient
 		private TRequest CovariantConverterWhenNeeded<T, TResult, TRequest, TRequestParameters>(RouteValues p, TRequest d)
 			where T : class
 			where TResult : class
-			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest 
+			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest
 			where TRequestParameters : IRequestParameters, new()
 		{
 			d.RequestParameters.DeserializationOverride = this.CreateSearchDeserializer<T, TResult, TRequest, TRequestParameters>(d);;
@@ -20,7 +20,7 @@ private TRequest CovariantConverterWhenNeeded<T, TResult, TRequest, TRequestPara
 		private Func<IApiCallDetails, Stream, SearchResponse<TResult>> CreateSearchDeserializer<T, TResult, TRequest, TRequestParameters>(TRequest request)
 			where T : class
 			where TResult : class
-			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest 
+			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest
 			where TRequestParameters : IRequestParameters, new()
 		{
 			CovariantSearch.CloseOverAutomagicCovariantResultSelector(this.Infer, request);
@@ -30,13 +30,16 @@ private Func<IApiCallDetails, Stream, SearchResponse<TResult>> CreateSearchDeser
 
 		private SearchResponse<TResult> FieldsSearchDeserializer<T, TResult, TRequest, TRequestParameters>(IApiCallDetails response, Stream stream, TRequest d)
 			where T : class
-			where TResult : class 
-			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest 
-			where TRequestParameters : IRequestParameters, new() =>
-			!response.Success
+			where TResult : class
+			where TRequest : IRequest<TRequestParameters>, ICovariantSearchRequest
+			where TRequestParameters : IRequestParameters, new()
+		{
+			var converter = new ConcreteTypeConverter<TResult>(d.TypeSelector);
+			var serializer = this.Serializer as JsonNetSerializer ?? new JsonNetSerializer(this.ConnectionSettings);
+			serializer.Initialize(converter);
+			return !response.Success
 				? null
-				: new JsonNetSerializer(this.ConnectionSettings, new ConcreteTypeConverter<TResult>(d.TypeSelector))
-					.Deserialize<SearchResponse<TResult>>(stream);
-
+				: serializer.Deserialize<SearchResponse<TResult>>(stream);
+		}
 	}
-}
+}

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

@@ -100,8 +100,10 @@ private SearchResponse<TResult> FieldsSearchDeserializer<T, TResult>(IApiCallDet
 			where TResult : class
 		{
 			var converter = this.CreateCovariantSearchSelector<T, TResult>(d);
+			var serializer = this.Serializer as JsonNetSerializer ?? new JsonNetSerializer(this.ConnectionSettings);
+			serializer.Initialize(converter);
 			var dict = response.Success
-				? new JsonNetSerializer(this.ConnectionSettings, converter).Deserialize<SearchResponse<TResult>>(stream)
+				? serializer.Deserialize<SearchResponse<TResult>>(stream)
 				: null;
 			return dict;
 		}

src/Tests/ClientConcepts/LowLevel/Connecting.doc.cs

@@ -275,11 +275,22 @@ public void ConfiguringSSL()
 		*/
 		public class MyJsonNetSerializer : JsonNetSerializer
 		{
-			public MyJsonNetSerializer(IConnectionSettingsValues settings) : base(settings) { }
+			private string _assignedInConstructor;
+			public MyJsonNetSerializer(IConnectionSettingsValues settings) : base(settings)
+			{
+				this._assignedInConstructor = "foo";
+			}
 
 			public int CallToModify { get; set; } = 0;
 
-			protected override void ModifyJsonSerializerSettings(JsonSerializerSettings settings) => ++CallToModify; //<1> Override ModifyJsonSerializerSettings if you need access to `JsonSerializerSettings`
+			public string MyProperty { get; set; }
+
+			//<1> Override ModifyJsonSerializerSettings if you need access to `JsonSerializerSettings`
+			protected override void ModifyJsonSerializerSettings(JsonSerializerSettings settings)
+			{
+				++CallToModify;
+				MyProperty = _assignedInConstructor;
+			}
 
 			public int CallToContractConverter { get; set; } = 0;
 
@@ -307,7 +318,7 @@ public void ModifyJsonSerializerSettingsIsCalled()
 			client.RootNodeInfo();
 			var serializer = ((IConnectionSettingsValues)settings).Serializer as MyJsonNetSerializer;
 			serializer.CallToModify.Should().BeGreaterThan(0);
-
+			serializer.MyProperty.Should().Be("foo");
 			serializer.SerializeToString(new Project { });
 			serializer.CallToContractConverter.Should().BeGreaterThan(0);
 		}

src/Tests/Document/Multiple/MultiGet/GetManyApiTests.cs

@@ -9,6 +9,8 @@
 using Tests.Framework.Integration;
 using Tests.Framework.MockData;
 using Xunit;
+using Elasticsearch.Net;
+using Newtonsoft.Json;
 
 namespace Tests.Document.Multiple.MultiGet
 {
@@ -52,5 +54,37 @@ public async Task UsesDefaultIndexAndInferredTypeAsync()
 				hit.Found.Should().BeTrue();
 			}
 		}
+
+		[U]
+		public void UsesCustomSerializationSettings()
+		{
+			const string expectedDateString = "2015-02-06T23:45:05Z";
+			var jsonResponse = $@"{{ ""docs"": [ {{ ""_id"": ""1"", ""_source"": {{ ""dateString"": ""{expectedDateString}"" }}}}]}}";
+
+			var connection = new InMemoryConnection(Encoding.UTF8.GetBytes(jsonResponse));
+			var connectionPool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
+			var connectionSettings = new ConnectionSettings(connectionPool, connection, settings => new LocalJsonNetSerializer(settings)).DefaultIndex("default-index");
+			var client = new ElasticClient(connectionSettings);
+
+			var hit = client.GetMany<HasDateString>(new[] { "1" })?.FirstOrDefault();
+			hit.Should().NotBeNull();
+			hit.Source.Should().NotBeNull();
+			hit.Source.DateString.Should().Be(expectedDateString);
+		}
+
+		private sealed class HasDateString
+		{
+			public string DateString { get; set; }
+		}
+
+		private sealed class LocalJsonNetSerializer : JsonNetSerializer
+		{
+			public LocalJsonNetSerializer(IConnectionSettingsValues settings) : base(settings) { }
+
+			protected override void ModifyJsonSerializerSettings(JsonSerializerSettings settings)
+			{
+				settings.DateParseHandling = DateParseHandling.None;
+			}
+		}
 	}
 }

src/Tests/Framework/TestClient.cs

@@ -54,11 +54,12 @@ public static IElasticClient GetFixedReturnClient(
 			string contentType = "application/json",
 			Exception exception = null)
 		{
-			var serializer = new JsonNetSerializer(new ConnectionSettings());
 			byte[] fixedResult;
 
 			if (contentType == "application/json")
 			{
+				var serializer = new JsonNetSerializer(new ConnectionSettings());
+				serializer.Initialize();
 				using (var ms = new MemoryStream())
 				{
 					serializer.Serialize(response, ms);