Mongo.Migration icon indicating copy to clipboard operation
Mongo.Migration copied to clipboard

Make DocumentVersion compatible with Json Deserialization.

Open thancock14 opened this issue 3 years ago • 2 comments

If the fields are not public properties they won't get used by either System.Text.Json or Netwonsoft.Json. This makes them still readonly, and compatible with deserialization of json without writing a custom JsonConverter.

thancock14 avatar Aug 12 '22 21:08 thancock14

public class myclass
{
    public Guid Id { get; set; }
    public DocumentVersion Version { get; set; }

}
public class Program
{
    public static async Task Main(string[] args)
    {
        var myclass = new myclass() { Version = new DocumentVersion("0.0.1"), Id = Guid.NewGuid() };


        var json = JsonSerializer.Serialize(myclass);
        var v = JsonSerializer.Deserialize<myclass>(json);

        //var json = JsonConvert.SerializeObject(myclass);
        //var v = JsonConvert.DeserializeObject<myclass>(json);

        Console.WriteLine(json);
        Console.WriteLine(v.Id);
        Console.WriteLine(v.Version);
    }
}

Here is my reproducible code.
If you run this you can see it serializes, but doesn't deserialize the DocumentVersion struct.
Nothing short of a Custom JsonConverter worked for me. but modifying the Struct to use properties works.

If you upgrade to c# 9 you can use get; init;  instead of get; set;

thancock14 avatar Aug 12 '22 21:08 thancock14

I went ahead and upgraded to c# 9.0, but had to include a package for init; Feel free to upgrade to .net 5 or 6 and it will just work without that package. I tried 5 and 6 and both work with your unit tests. not sure if you need 3.1 for anything since it's not supported anymore

thancock14 avatar Aug 12 '22 21:08 thancock14

Coverage Status

Coverage increased (+0.07%) to 80.123% when pulling be2bef0f4bec3137d4c43af2cbe888ad4af4b200 on thancock14:StructJsonCompatible into ef35ca65d04c9f93f757682dd288331890bee7b9 on SRoddis:master.

coveralls avatar Sep 05 '22 12:09 coveralls

Thanks! will you be able to push a nuget update for this? thanks!

thancock14 avatar Sep 20 '22 14:09 thancock14

@SRoddis could you get this kicked to nuget for me? I could really use this fix in our project

thancock14 avatar Jan 10 '23 17:01 thancock14