Support JsonSerializerOptions options in Copy operation

vahoang commented 2 years ago

Description

Currently only the Copy operation at here does not use the provided JsonSerializerOptions.

To Reproduce

Create a custom converter (ex: a converter to support new type DateOnly)

Add the converter in the JsonSerializerOptions options

JsonSerializerOptions jsonOptions = new JsonSerializerOptions()
{
Converters =
{
  new JsonPatchDocumentConverterFactory(),
  // below is the custom converter
  new DateOnlyConverter(),
},
};

Try to use json patch document that will trigger the custom converter (ex: in this case use model with DateOnly attribute)
All other operations work fine except Copy.

Tasks

[ ] Pass the provided JsonSerializerOptions options to ConversionResultProvider.TryCopyTo
[ ] Use the options when try to deserialize the object in TryCopyTo method
[ ] Write test cases

vahoang commented 2 years ago

@Havunen Can help me double check if the issue is valid and if I can help you to fix it, many thanks!!

Havunen commented 2 years ago

Yeah, I think it should ideally use the provided options in those Serialize methods, but there was some issue where the logic got into infinite loop because it was calling itself. That needs to be tested.

vahoang commented 2 years ago

@Havunen Do you recall how did you end up with such infinite loop cause I did some tests and cannot reproduce it.

Havunen commented 2 years ago

Ah, it seems TryConvertTo was already passing the json serializer options there, I believe I fixed it at some point then. Yeah this sounds like a bug

Havunen commented 2 years ago

Fixed in v0.0.9

vahoang commented 2 years ago

Thanks for the swift replies and fix !!

Havunen / SystemTextJsonPatch