JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.84k stars 3.26k forks source link

[bug] Serialize same instance with converter in two lists using reference and type name causes duplicated id #570

Closed yume-chan closed 9 years ago

yume-chan commented 9 years ago

I'm using 7.0.1b3 on .NET and 6.0.8 on Portable Library.

// I Want to serialize my complex data:
class ContentB { public bool SomeValue { get; set; } }

[JsonConverter(typeof(ListConverter))]
class ContentA : List<object>
{
    public ContentB B { get; set; } = new ContentB();
}

// It's a List<object> so I need to serialize it with a converter:
class ListConverter : JsonConverter
{
    public override bool CanConvert(Type objectType)
    {
        return true;
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        return new ContentA() { B = serializer.Deserialize<ContentB>(reader) }; // Construct my data back.
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        serializer.Serialize(writer, (value as ContentA).B); // My Content.B contains all useful data.
    }
}

// My data will be in a container, container have several lists may contain a same instance. 
class Container
{
    public List<ContentA> ListA { get; set; }
    public List<ContentA> ListB { get; set; }

    public Container()
    {
        ListA = new List<ContentA>();
        ListA.Add(new ContentA());

        ListB = new List<ContentA>();
        ListB.Add(ListA[0]); // A instance is in both two lists.
    }
}

// Now I'm serializing it, my complex data may have many types so I want to include references and type names.
string Serialize()
{
    var settings = new JsonSerializerSettings();
    settings.PreserveReferencesHandling = PreserveReferencesHandling.All;
    settings.ReferenceLoopHandling = ReferenceLoopHandling.Serialize; // JSON.Net thinks I'm creating loop references, just make it happy.
    settings.TypeNameHandling = TypeNameHandling.All;
    settings.Formatting = Formatting.Indented;

    return JsonConvert.SerializeObject(new Container(), settings);
}

// Result:
/*
{
    "$id": "1",
    "$type": "ConsoleApplication1.Container, ConsoleApplication1",
    "ListA": {
        "$id": "2",
        "$type": "System.Collections.Generic.List`1[[ConsoleApplication1.ContentA, ConsoleApplication1]], mscorlib",
        "$values": [
            {
                "$id": "3",
                "$type": "ConsoleApplication1.ContentB, ConsoleApplication1",
                "SomeValue": false
            }
        ]
    },
    "ListB": {
        "$id": "4",
        "$type": "System.Collections.Generic.List`1[[ConsoleApplication1.ContentA, ConsoleApplication1]], mscorlib",
        "$values": [
            {
                "$id": "3",
                "$type": "ConsoleApplication1.ContentB, ConsoleApplication1",
                "SomeValue": false
            }
        ]
    }
}
*/

// What? two objects with same id=3?

// Try to deserialize it:
Container Deserialize(string content)
{
    var settings = new JsonSerializerSettings();
    settings.PreserveReferencesHandling = PreserveReferencesHandling.All;
    settings.ReferenceLoopHandling = ReferenceLoopHandling.Serialize; // JSON.Net thinks I'm creating loop references, just make it happy.
    settings.TypeNameHandling = TypeNameHandling.All;
    settings.Formatting = Formatting.Indented;

    return (Container)JsonConvert.DeserializeObject(content, settings);
}

// An exception is thrown:
/*
Newtonsoft.Json.JsonSerializationException: Error reading object reference '3'. Path 'ListB.$values[0].SomeValue', line 22, position 21.
    ---> System.ArgumentException: A different value already has the Id '3'.
        at Newtonsoft.Json.Utilities.BidirectionalDictionary`2.Set(TFirst first, TSecond second)
        at Newtonsoft.Json.Serialization.DefaultReferenceResolver.AddReference(Object context, String reference, Object value)
        at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.AddReference(JsonReader reader, String id, Object value)
        --- End of inner exception stack trace ---
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.AddReference(JsonReader reader, String id, Object value)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateObject(Object newObject, JsonReader reader, JsonObjectContract contract, JsonProperty member, String id)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateObject(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.CreateValueInternal(JsonReader reader, Type objectType, JsonContract contract, JsonProperty member, JsonContainerContract containerContract, JsonProperty containerMember, Object existingValue)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.Deserialize(JsonReader reader, Type objectType, Boolean checkAdditionalContent)
    at Newtonsoft.Json.Serialization.JsonSerializerProxy.DeserializeInternal(JsonReader reader, Type objectType)
    at Newtonsoft.Json.JsonSerializer.Deserialize[T](JsonReader reader)
    at ConsoleApplication1.ListConverter.ReadJson(JsonReader reader, Type objectType, Object existingValue, JsonSerializer serializer)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.DeserializeConvertable(JsonConverter converter, JsonReader reader, Type objectType, Object existingValue)
    at Newtonsoft.Json.Serialization.JsonSerializerInternalReader.PopulateList(IList list, JsonReader reader, JsonArrayContract contract, JsonProperty containerProperty, String id)
*/

If I didn't use converter JSON.NET will create correct references, if I didn't include type names JSON.NET can deserialize it without problem, but if I combine them the problem will occur.

JamesNK commented 9 years ago

Fixed

yume-chan commented 9 years ago

My class overrode Equals() method so a ContentA and a ContentB will be equivalent if their have same data.

[JsonConverter(typeof(ListConverter))]
class ContentA {
    public ContentB B { get; set; }
    public override bool Equals(object other) {
        // ......
        if (other is ContentB)
            return this.B.Equals(other);
        // ......
    }

But JSON.NET thinks it's a self reference because JsonSerializerInternalWriter._serializeStack already contained a ContentA which is equals to the converted ContentB.

Maybe JsonSerializerInternalWriter.SerializeConvertable() should not add the source object to _serializeStack (Now it doesn't add the converted object, is it another bug)? Or maybe JsonSerializerInternalWriter.CheckForCircularReference() should check equability by ReferenceEquals()?

In fact after this fix my code can never finish, I'm trying to build a minimal code example. EDIT: sorry, my code has other true loop references.