Search code examples
c#json.net

Newtonsoft json has a deserialization bug when class has 'quick' access property for list


I have parent class that contains a collection of child POCOs as well as a property that provides quick access to the first member of the collection. The child POCO types in turn contain another child collection. When I round-tip the parent using Json.NET, the contents of the first child's collection get duplicated.

What can be done to prevent this?

Minimal reproducible as follows:

using System;
using Newtonsoft.Json;
using System.Collections.Generic;
using System.Linq;

namespace MyApp // Note: actual namespace depends on the project name.
{
    internal class Program
    {
        static void Main(string[] args)
        {
            var parent = new Parent
            {
                Children = new List<Child>
                {
                    new Child
                    {
                       Children = new List<GrandChild>
                        {
                            new GrandChild { Value = "I am a grand child" },
                        }
                    },
                    new Child(),
                }
            };

            var serializedParent = JsonConvert.SerializeObject(parent);
            var deserializedParent = JsonConvert.DeserializeObject<Parent>(serializedParent);

            // Expected: 1, Actual: 2
            Console.WriteLine(deserializedParent.Children[0].Children.Count);
        }
   }

    public class Parent
    {
         public List<Child> Children { get; set; } = new List<Child>();

         public Child FirstChild => Children.First();
    }

    public class Child
    {
        public List<GrandChild> Children { get; set; } = new List<GrandChild>();
    }

    public class GrandChild
    {
        public string Value { get; set; }
    }
}

Demo fiddle with a failing assertion here.


Solution

  • Your problem is that Json.NET will populate a get-only property when it is pre-allocated. In the case of the FirstChild you need to avoid this, because, in doing so, Json.NET ends up deserializing the contents of FirstChild.Children twice.

    In detail, the sequence of events during deserialization is as follows:

    1. Since "Children" property happens to come first in the JSON, the Children array is completely deserialized.

    2. At this point, FirstChild now returns a Child instance rather than throwing a System.InvalidOperationException: Sequence contains no elements exception.

    3. Json.NET now encounters the "FirstChild" property in the JSON, binds it to the Parent.FirstChild c# property. Since it is non-null, it populates the contents, which means that FirstChild.Children is populated twice as explained in Json.net deserializing list gives duplicate items.

    So, what are your workarounds?

    Firstly, if you don't need FirstChild in the JSON, you could simply mark it with [JsonIgnore] which will prevent it from being serialized or deserialized:

    public class Parent
    {
        public List<Child> Children { get; set; } = new List<Child>();
    
        [JsonIgnore]
        public Child FirstChild => Children.First();
    }
    

    This will also prevent serialization from failing when Children is empty. Demo fiddle here.

    Secondly, if you must have FirstChild present in your JSON, you could attach a JsonConverter to the property that, in ReadJson(), skips the incoming JSON and simply returns the existing value:

    public class Parent
    {
        public List<Child> Children { get; set; } = new List<Child>();
    
        [JsonConverter(typeof(SerializeOnlyJsonConverter))]
        public Child FirstChild => Children.First();
        
        public bool ShouldSerializeFirstChild() => Children != null && Children.Count > 0;
    }
    
    public class SerializeOnlyJsonConverter : JsonConverter
    {
        public override bool CanConvert(Type objectType) => throw new NotImplementedException("This converter should only be applied directly to properties");
    
        public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) 
        {
            reader.Skip();
            return existingValue;
        }
        
        public override bool CanWrite => false;
        public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) => throw new NotImplementedException();
    }
    

    You will also need to add a ShouldSerialize method to prevent serializing from failing when Children is null or empty. Or you could fix FirstChild to never throw an exception:

        public Child FirstChild => Children?.FirstOrDefault();
    

    Demo fiddle #2 here.

    Thirdly, you could serializing using GetOnlyContractResolver from this answer by Pavlo Lissov to Serialize Property, but Do Not Deserialize Property in Json.Net and apply [GetOnlyJsonProperty] to FirstChild

    public class Parent
    {
        public List<Child> Children { get; set; } = new List<Child>();
    
        [GetOnlyJsonProperty]
        public Child FirstChild => Children.First();
        
        public bool ShouldSerializeFirstChild() => Children != null && Children.Count > 0;
    }
    
    [System.AttributeUsage(System.AttributeTargets.Property, AllowMultiple = false)]
    public class GetOnlyJsonPropertyAttribute : Attribute
    {
    }
    
    public class GetOnlyContractResolver : DefaultContractResolver
    {
        // From this answer https://stackoverflow.com/a/56885301/3744182
        // By https://stackoverflow.com/users/7027460/pavlo-lissov
        // To https://stackoverflow.com/questions/31731320/serialize-property-but-do-not-deserialize-property-in-json-net
        protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
        {
            var property = base.CreateProperty(member, memberSerialization);
            if (property != null) 
            {
                var attributes = property.AttributeProvider.GetAttributes(typeof(GetOnlyJsonPropertyAttribute), true);
                if (attributes != null && attributes.Count > 0)
                    property.ShouldDeserialize = (a) => false;  
            }
            return property;
        }
    }
    

    And then serialize as follows:

    IContractResolver resolver = new GetOnlyContractResolver(); // In production code this should be cached statically to improve performance
    var settings = new JsonSerializerSettings { ContractResolver = resolver };
    
    var serializedParent = JsonConvert.SerializeObject(parent, settings);
    var deserializedParent = JsonConvert.DeserializeObject<Parent>(serializedParent, settings);
    
    var reserializedParent = JsonConvert.SerializeObject(deserializedParent, settings);
    

    Demo fiddle #3 here.