Use custom connection serialization settings, if available. #2182
Conversation
|
Hi @mitchknife, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile? |
|
Ok, updated my profile with the new email. |
|
@mitchknife, cool, the CLA is passing now. |
|
Hey @mitchknife thanks for the PR. This is definitely an issue, but I'm not quite sold on this solution because it doesn't protect against the case where The problem stems from the fact that we have a custom MultiGetConverter that disregards any registered custom Will think of a solution and open a new PR for this, but huge 👍 for raising this. There maybe other instances (other endpoints) that suffer from this as well, so we also need to do a pass over all the endpoints that use custom converters in this fashion. |
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
|
@mitchknife see the attached PR above. |
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
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
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
This solves a problem similar to #2155 but for multi-get. My date strings were not round tripping correctly.
Digging into it I found that my custom
JsonSerializationSettingswere being ignored inJsonNetSerializer(DateParseHandling.Nonespecifically) during a multi-get call.This is more of a comprehensive solution than #2155 since the problem looks to be a bit more systemic than it did at first.
I was unable to run all tests at once (before and after the change), but ran a number of them individually.