Newtonsoft.Json icon indicating copy to clipboard operation
Newtonsoft.Json copied to clipboard

Class objects that inherit from the collection interface, all or part of the serialized data is lost

Open ghost opened this issue 1 year ago • 7 comments

Source/destination types


public class BadClass : List<int>
{
    public List<int> List { get; set; }

    public string MyProperty { get; set; }

    public BadClass(string myProperty, List<int> list)
    {
        MyProperty = myProperty;
        List = list;
    }
}

public class NiceClass
{
    public List<int> List { get; set; }

    public string MyProperty { get; set; }

    public NiceClass(string myProperty, List<int> list)
    {
        MyProperty = myProperty;
        List = list;
    }
}

var nice = new NiceClass("Hello World", new List<int>() { 1, 2, 3 });
var bad = new BadClass("Hello World", new List<int>() { 1, 2, 3 });

Source/destination JSON


nice:
{"List":[1,2,3],"MyProperty":"Hello World"}
bad:
[]

Expected behavior

I can easily serialize objects if I don't use inheritance, but then I don't get to use the benefits that the interface gives me. Is there a better solution that does not impose too much additional burden? I do not want to change the class to solve the problem, because it maybe make some bugs or change more code.

If call 'Add(item)' in BadClass, will get data: [1,2,3]. I don't know whether define 'List' property for saving items.

Actual behavior

Steps to reproduce

ghost avatar May 08 '23 09:05 ghost

Your BadClass class derives from List<T>, and Newtonsoft.Json therefore treats it as such and attempts to serialize it into a json array. But since your BadClass list instance in your example code above does not contain any items, the resulting json array is empty [].

Since your BadClass class derives from List<T> and therefore can/will contain any items at some point, what is the desired serialization result when a BadClass instance contains items? Asked differently, for something like this:

var example = new BadClass("Hello World", new List<int>() { 1, 2, 3 });
example.Add(7);
example.Add(8);
example.Add(9);

what should the desired json look like? Depending on the desired outcome, you might choose either a custom JsonConverter or a custom ContractResolver (or perhaps even just some attribute annotations on the BadClass, if the situation turns out to be simple...)


(P.S.: List<int> is not an interface. It is a class. Therefore BadClass is inheriting from a class.)

elgonzo avatar May 08 '23 11:05 elgonzo

I want to get data: {"Items":[1,2,3],"MyProperty":"Hello World"}.

Being able to get List<int> data in some way is my requirement. My understanding is that Json does not have the flexibility to control the form of expression and still has certain requirements for data structure. So defining the List property to store the data seems to be the way to go, otherwise you need to construct the object via JsonObject.

If I define and store it in List, then I think JsonConvert should have the responsibility to help me identify it, and it seems to be handled in a less flexible and convenient way. whether it is interface inheritance or class inheritance, it should not be seen as simply a collection class, the real array is List, Array, these types of instances, note that it is not the type after inheritance.

By the way, it seems that you should not use a property to receive objects that inherit from a List, but let JsonConvert read it by reflection into a custom property "Items" that you add by default. Because if you define a property ‘MyList’ to receive data, then all methods should be overridden to update the data and ensure consistency. So I defined an additional property to handle what is itself a bad practice.

But inheritance is not guilty, whenever it is correct, just need to pay attention to try to do some extra complex things, otherwise you have to do a lot of rewrite methods to achieve synchronous data updates. For example, if you define another MyList yourself, then you have to update MyList synchronously in Add and Remove so that the Count is correct and all behavior is normal and not bothered. Try not to do it though, because the data is too redundant and weird, and if you do, JsonConvert should be able to support it. Because it doesn't rewrite itself there are just some design flaws of its own or just for extra caching of data, not involved in maintenance or synchronization of related information.

If the user has defined Items Property, this time the base class collection is also stored with Items, they conflict, this time when the serialization throws an exception. This can be solved by letting the user specify an alias or modify the properties.

Some people might suggest that I use an interface type, let's say IList, but this is clumsy and I need to rewrite a lot of methods. In this case, I just don't need to go the extra mile of defining the property MyList to store it to be done conveniently, and the data members are initialized by the Add method. Hopefully, I can improve the logic flaws of serialization.

I hope I have expressed myself clearly and unambiguously, and hopefully this problem can be solved very well, perhaps a flaw.

ghost avatar May 09 '23 01:05 ghost

Do i understand you correctly that the public List<int> List { get; set; } property in your BadClass is only an attempt to work around the serialization issue, but this property has otherwise no purpose and you would prefer not having this property?

elgonzo avatar May 09 '23 06:05 elgonzo

Yes. Serialization does not currently support this feature, so I was forced to modify the class to remove the inheritance from a List class.

ghost avatar May 09 '23 08:05 ghost

So, if i understand correctly, your actual problematic classes look more like:

public class ActualClass : List<int>
{
    public string SomeProperty { get; set; }
    public int SomeOtherProperty { get; set; }

    public ActualClass(string someProperty, int someOtherProperty)
    {
        SomeProperty = someProperty ;
        SomeOtherProperty = someOtherProperty ;
    }
}

inheriting from List<T> while also defining one or multiple properties of whatever types.

That should be doable with a custom ContractResolver only. The advantage of doing this only through a contract resolver is that the serializer still keeps adhering to any serialization-related attributes and settings Newtonsoft.Json support. (Using a customer JsonConverter on the other hand would put the burden of adhering to serialization settings and attributes applying to members of the de/serialized types on the converter itself).

The contract resolver i outline below should hopefully not be that complicated to follow and relaitvely easy to adapt to your actual situation; if you have questions about what the properties/types/methods used in this contract resolver are, i'd like to refer you to the Newtonsoft.Json documentation, especially to the API documentation for DefaultContractResolver and JsonProperty

First, the contract resolver needs to be able to identify types that derive from List<T> (such as ActualClass) and get the concrete List<T> type inherited:

public class InheritedFromListOfTContractResolver : DefaultContractResolver
{
    //
    // Returns the concrete List<T> type if the given type inherits from some List<T>,
    // otherwise returns null.
    //
    private static Type? GetListTypeIfInheritingFromListOfT(Type t)
        => (t.BaseType is null) ? null
            : (t.BaseType.IsGenericType && t.BaseType.GetGenericTypeDefinition() == typeof(List<>)) ? t.BaseType
            : GetListTypeIfInheritingFromListOfT(t.BaseType);
    ...

With that, we can make the contract resolver create a json object contract (instead of the default json array contract) for the desired List<T>-inheriting types (look up the documentation for this overridden method in the DefaultContractResolver API documentation i mentioned above):

    public override JsonContract ResolveContract(Type type)
        => (GetListTypeIfInheritingFromListOfT(type) is not null)
            ? CreateObjectContract(type)
            : base.ResolveContract(type);
    ...

But we still need to customize that json object contract in two ways. First, List<T> itself declares two public properties that don't need (i guess) to be serialized : Count and Capacity. To simplify the logic and not hard-code those property names in the method code, lets put these property names in a HashSet for easy lookup:

    private readonly HashSet<string> _listPropertyNamesToIgnore = new(
        typeof(List<>).GetProperties().Select(pi => pi.Name)
    );
    ...

Besides excluding these properties from serialization, we also need to add another new json property for the json array with the items hold in the List<T>-derived instance being serialized. I did choose the name <items> (including the angled brackets), as it cannot conflict with any legal C# property name.

For both ignoring the List<T> properties and adding our own items properties, we override the CreateProperties method (again, look up the documentation for this overridden method in the DefaultContractResolver API documentation i mentioned above):

    protected override IList<JsonProperty> CreateProperties(Type type, MemberSerialization memberSerialization)
    {
        var properties = base.CreateProperties(type, memberSerialization);
        var inheritedListType = GetListTypeIfInheritingFromListOfT(type);

        if (inheritedListType is not null)
        {
            //
            // Exclude properties declared by List<T> from de/serialization
            //
            foreach (var p in properties.Where(p => _listPropertyNamesToIgnore.Contains(p.UnderlyingName)))
            {
                p.Readable = false;
                p.Writable = false;
            }

            //
            // Add a property for the json array containing the items of the List<T>-derived instance
            //
            var itemsListProperty = new JsonProperty
            {
                PropertyName = "<items>",
                PropertyType = inheritedListType,
                ValueProvider = new ListItemsProvider(inheritedListType),
                Readable = true,
                Writable = true
            };
            properties.Add(itemsListProperty);
        }

        return properties;
    }
    ...

One thing to note here is the ValueProvider property of the JsonProperty we create for the items json array. We have to tell the serializer how to get and set this property during serialization and deserialization. This is done through the IValueProvider interface (https://www.newtonsoft.com/json/help/html/T_Newtonsoft_Json_Serialization_IValueProvider.htm), which we implement as follow:

    private class ListItemsProvider : IValueProvider
    {
        internal ListItemsProvider(Type listType)
        {
            this._listType = listType;
        }

        public object GetValue(object target)
            => Activator.CreateInstance(_listType, target);

        public void SetValue(object target, object value)
        {
            IList targetInstance = (IList) target;
            foreach (var item in (IList)value)
                targetInstance.Add(item);
        }

        private readonly Type _listType;
    }

Note how the CreateProperties method above passes the concrete List<T> type to the ListItemsProvider. This is done so the value for the items collection returned by GetValue is not the instance type being represented by the contract for the serialization process. Just using the original object instance here would create a self-referencing object graph that the serializer would detect and respond to with an exception (unless you configure the serializer to preserve references, but that is an entirely different topic and would not yield the result you want, based on my understanding).

Also note how the SetValue method (used for setting/assigning the deserialized value to the target C# object) doesn't need to know anything about the concrete List<T> type. For adding the items to the target object we can just use the non-generic IList interface which List<T> implements. The deserializer itself knows the concrete List<T> type necessary for correct deserialization especially of numerical values thanks to our CreateProperties method setting the PropertyType of the <items> json property accordingly, saving our IValueProvider implementation from otherwise possibly required value type conversion shenanigans.

And that's it. All that's left is to setup the serializer to use this contract resolver via the JsonSerializerSettins.ContractResolver property...

A simple example demonstrating both serialization and deserialization (including parameterized constructor and get-only properties): https://dotnetfiddle.net/0loKxM

A somewhat more complex example demonstrating that serialization-related attributes (like [JsonIgnore], for example) and serialization settings will still be honoured by the serializer (fyi: something that would require orders of magnitude more code/work when using a custom JsonConverter instead of exploiting the capabilities of DefaultContractResolver): https://dotnetfiddle.net/nVUrfd Note how the first Data item collapses into an empty json object, and both PropertyBeingZero and PropertyBeingIgnored properties of ActualClass not being present in the json data due to the [JsonIgnore] attributes and the DefaultValueHandling.Ignore setting.


If you have managed to read all the stuff until here, for the end something to keep in the back of your mind (not really an advice but more of a reminder, if you will). Having to serialize/deserialize a type that both combines aspects of what would naturally be a json object and other aspects of what would naturally be a json array isn't really that common, as the lack of a mechanism in many (most???) json serializers and lack of a standard json representation for such should tell (at least not so common to compel writers of json serializers to generally support such out-of-the-box).

While the custom contract resolver i illustrated here is addressing your immediate problem (as far as i understand it), it might perhaps be worthwhile to contemplate if it would be better in the long run to avoid these kind of class declarations that are both collection types and have custom properties which need to be preserved in the serializated json data. Because if you avoid such types, you therefore avoid this problem and therefore won't have this struggle again if you ever want or have to use another json serializer. But that's of course not for me to say (why would i, i have no stake in what you do :smile:), it's is entirely your own decision to make. But it's worth pointing out the elephant in the room in that json is a popular data exchange format for give-or-take 17 years by now, and after all these years there is by-large no commonly agreed-upon "standard"/typical representation for the data structure you want/need to serialize.

elgonzo avatar May 09 '23 13:05 elgonzo

Just an addendum (and not strictly necessary). It just occured to me that the ListItemsProvider's GetValue method doesn't really need the concrete list type, because it could just create a non-generic object collection, albeit at the expense of boxing of value types (shouldn't matter much, though):

    private class ListItemsProvider : IValueProvider
    {
        public object GetValue(object target)
            => new ArrayList((IList) target);

        public void SetValue(object target, object value)
        {
            IList targetInstance = (IList) target;
            foreach (var item in (IList)value)
                targetInstance.Add(item);
        }
    }

elgonzo avatar May 09 '23 16:05 elgonzo

You should understand it very well, I did not read it too well. For the time being, I'd settle for the simplest and most straightforward way to solve this kind of problem, not to inherit from collection interfaces or classes.

ghost avatar May 10 '23 03:05 ghost