Newtonsoft.Json

Newtonsoft.Json copied to clipboard

When having multiple JsonConverters defined in the Converters property of JsonSerializer, the converter chosen for serialization is the first JsonConverter where CanConvert returns true for the type, regardless of whether CanWrite is true. If CanWrite is false, the default serialization will be used, even if there are JsonConverters further down the list that could handle serialization of the type.

This is probably working as defined, however a more sensible behavior (in my opinion) would be to ignore that JsonConverter and fall through and check if the next one can handle serialization for the type. Otherwise, it is impossible to have one JsonConverter that handles serialization and another that handles deserialization for the same type.

Source/destination types

Any type that has at least 2 custom JsonConverters defined for it, where the first one has CanWrite=false and the second (or later) has CanWrite=true.

Source/destination JSON

JSON generated by custom JsonConverter

Expected behavior

Using the first JsonConverter where CanConvert returns true for the type, and where CanWrite returns true. The Converters list should be iterated until a JsonConverter is found that returns true for both.

Actual behavior

The first JsonConverter where CanConvert returns true for the type is selected, regardless of what CanWrite returns. If CanWrite is false, the default serialization is invoked.

Steps to reproduce

public class CustomConverterA : JsonConverter
{
    public override bool CanWrite
    {
        get
        {
            Console.WriteLine("Inside CustomConverterA.CanWrite (returns false)");
            return false;
        }
    }
    public override bool CanRead => true;

    public override bool CanConvert(Type objectType)
    {
        Console.WriteLine("Inside CustomConverterA.CanConvert");

        if (objectType == typeof(string))
        {
            Console.WriteLine("return true");
            return true;
        }

        Console.WriteLine("return false");
        return false;
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        Console.WriteLine("Inside CustomConverterA.WriteJson");
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        Console.WriteLine("Inside CustomConverterA.ReadJson");

        return new object();
    }
}

public class CustomConverterB : JsonConverter
{
    public override bool CanWrite
    {
        get
        {
            Console.WriteLine("Inside CustomConverterB.CanWrite (returns true)");
            return true;
        }
    }
    public override bool CanRead => true;

    public override bool CanConvert(Type objectType)
    {
        Console.WriteLine("Inside CustomConverterB.CanConvert");

        if (objectType == typeof(string))
        {
            Console.WriteLine("return true");
            return true;
        }

        Console.WriteLine("return false");
        return false;
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        Console.WriteLine("Inside CustomConverterB.WriteJson");
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        Console.WriteLine("Inside CustomConverterB.ReadJson");

        return new object();
    }
}

public void TestCustomConverter()
{
    var serializer = new JsonSerializer()
    {
        Converters = {
            new CustomConverterA(),
            new CustomConverterB()
        }
    };

    var data = "testdata";

    using (var writer = new StringWriter())
    {
        serializer.Serialize(writer, data);
        var serialized = writer.ToString();
        Console.WriteLine("Serialized: " + serialized);
    }
}

The above outputs:

Inside CustomConverterA.CanConvert return true Inside CustomConverterA.CanWrite (returns false) Serialized: "testdata"

I would expect CustomConverterB to be chosen instead.

@JamesNK - do you think this functionality is worth adding? I'm happy to give it a go, doesn't look like it's likely to break very much (e.g. adding a "forWriting" parameter Serializer.GetMatchingConverter). OTOH it would still potentially introduce changes in client behaviour and for questionable gain...

@wizofaus I don't agree that the gain is questionable. Right now it is impossible to use different converters for reading and writing, which is very useful if, say, you want to inherit writing behavior from one converter and reading behavior from another. Since multiple inheritance is not allowed in C# the only way to solve it is to duplicate code or to have the code in a separate class that is then called by thin-wrapper converters. Not as elegant in my eyes.

When it comes to backward compatibility I think it could be solved by an option on either the converter or the serializer, that defaults to the old behavior.

Adding myself to the list of people this issue affects.

Me also.

Me too 😞

While I think this scenario should be supported directly it's quite trivial to create a wrapper converter that delegates to each, something like this.

public sealed class ReadWriteJsonConverter : JsonConverter
{
    public JsonConverter Reader { get; }
    
    public JsonConverter Writer { get; }
    
    public ReadWriteJsonConverter(JsonConverter reader, JsonConverter writer)
    {
        Reader = reader;
        Writer = writer;
    }
    
    public override bool CanConvert(Type objectType) => Reader.CanConvert(objectType);

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        return Reader.ReadJson(reader, objectType, existingValue, serializer);
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        Writer.WriteJson(writer, value, serializer);
    }
}

While I think this scenario should be supported directly it's quite trivial to create a wrapper converter that delegates to each, something like this.

Ah, yes, that's what I meant by "thin-wrapper converters". Thanks for providing an example!

If not for determining whether to serialize with a given converter, what is CanWrite actually used for?

The documentation is vague on this, seeing as how the intuitive meaning of the property isn't correct.

As far as I know Json.NET will use the default serialization behavior when CanWrite is false and thus not call the WriteJson method of the converter. Similarly I believe Json.NET will use the default deserialization behavior when CanRead is false and thus not call the ReadJson method of the converter.