Inconsistent Json Converter behaviour

[shvez]

We encountered weired behaviour or Json converter on both .net8 and .netframework4.xxx using Newtonsoft 13.0.3

we have simple class like this and json string for it:

var jsonString =
    "{\"ClosedParameters\":{\"4\":{\"key1\":1,\"key2\":\"value2\",\"key3\":[1,2,3]}}}";

public class Ticket
{
    public Dictionary<byte, object> ClosedParameters { get; set; }= new Dictionary<byte, object>();
}

if we call JsonConvert.DeserializeObject<Ticket> for jsonString then Converter is called only once for evey dictionary entry.
Next I call 'JsonConvert.SerializeObject' for ticket instance and then again JsonConvert.DeserializeObject<Ticket> for the same jsonString converter is called also for 'key1', 'key2' and 'key3'.

I do not know what is right behaviour here. I would like to see second one. For me it is not clear what I do wrong and get different behaviour although input and serialization settings are same

What should I fix to get it working same way eveytime?

Here is json

{
  "ClosedParameters": {
    "4": {
      "key2": "value2",
      "key3": [
        1,
        2,
        3
      ],
      "key1": 1
    }
  }
}

Here is simple console app with converter that demonstrating the issue

using System.Collections;
using Newtonsoft.Json;

Console.WriteLine("----------------------\n  First Call output\n----------------------");

var jsonString =
    "{\"ClosedParameters\":{\"4\":{\"key1\":1,\"key2\":\"value2\",\"key3\":[1,2,3]}}}";


var deserializeSettings = new JsonSerializerSettings
{
    Converters = new List<JsonConverter> { new ClosedParametersConverter() },
    NullValueHandling = NullValueHandling.Ignore,
    DateParseHandling = DateParseHandling.None
};

var t = JsonConvert.DeserializeObject<Ticket>(jsonString, deserializeSettings);

var ticket = new Ticket()
{
    ClosedParameters = new Dictionary<byte, object>()
    {
        { 4, new Hashtable
            {
                {"key1", 1},
                {"key2", "value2"},
                {"key3", new object[] { 1, 2, 3 } }
            }
        }
    }
};

var json = JsonConvert.SerializeObject(ticket, new JsonSerializerSettings
{
    NullValueHandling = NullValueHandling.Ignore,
    DateParseHandling = DateParseHandling.None
});


Console.WriteLine("----------------------\n  Serialization Result.\n----------------------");

Console.WriteLine(json);

Console.WriteLine("----------------------\n  Second call output.\n----------------------");


var ticket2 = JsonConvert.DeserializeObject<Ticket>(jsonString, deserializeSettings);



return;

public class Ticket
{
    public Dictionary<byte, object> ClosedParameters { get; set; }= new Dictionary<byte, object>();
}

public class ClosedParametersConverter : JsonConverter
{
    private bool conversion;
    private int hashtableStarted;

    private int recursionLevel = 0;

    public override bool CanConvert(Type objectType)
    {
        if (this.conversion && objectType == typeof(object))
        {
            return true;
        }

        if (objectType == typeof(Dictionary<byte, object>))
        {
            this.conversion = true;
        }

        return false;
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        Console.WriteLine($"ReadJson: {reader.TokenType} {reader.ValueType} {reader.Value} {reader.Depth} {reader.Path}");

        if (++this.recursionLevel >= 50)
        {
            throw new InvalidOperationException("Recursion level exceeded");
        }

        try
        {
            if (reader.Path.Contains("ClosedParameters"))
            {
                switch (reader.TokenType)
                {
                    case JsonToken.StartObject:
                        ++this.hashtableStarted;
                        var r = serializer.Deserialize(reader, typeof(Hashtable));

                        --this.hashtableStarted;
                        return r;
                    case JsonToken.Integer:
                        if (this.hashtableStarted > 0)
                        {
                            return serializer.Deserialize(reader, typeof(int));
                        }

                        return serializer.Deserialize(reader, reader.ValueType);
                    case JsonToken.StartArray:
                        if (this.hashtableStarted > 0)
                        {
                            var o = serializer.Deserialize(reader, typeof(object[]));

                            return o;
                        }

                        return serializer.Deserialize(reader, reader.ValueType);
                    default:
                        return serializer.Deserialize(reader, reader.ValueType);
                }
            }

            this.conversion = false;

            return serializer.Deserialize(reader, objectType);
        }
        finally
        {
//                --this.recursionLevel;
        }
    }

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

[shvez]

After some debugging I found out that internally you have contract's cache. During very first deserialization of Hashtable you create contract for it with 'Item' and 'Key' contracts set to 'null', although I would expect that they should not be null.

After serialization that contract is updated and has Key- and Item- contracts set to JsonObjectContract. after this update my json converter is called as expected.

Current workaround for me to call SerializeObject(new Hashtable{......,

is there better way to fix the issue?

[elgonzo]

Normally the expectation is that a converter's CanConvert is only called once for given type -- which determines whether the converter is applicable to that type or not. The whole logic in your converter regarding this.conversion looks very iffy to me, appearing to be based on the shaky assumption that a converter's CanConvert method will be called multiple times for the same type...

[shvez]

well, dictionary is not a goal of this converter. its goal is find json object and convert it to Hashtable with some extra conversion rules. I do not expect that CanConvert will be called many times for same type.
I did expect that for fields of Hashtable ReadJson will be called.
I agree, implementation might be ugly, because I do not have deep understand of concepts behind this lib but it does what we need

[elgonzo]

If i am not mistaken (but i could be), it looks like you attempted to write a converter for the object items of the dictionary. If that's the case, you can make your life easier without needing to resort to iffy hacks like the this.conversion flag tracking whether the deserializer is currently in the process of deserializing a Dictionary<byte, object>.

Write your own contract resolver deriving from DefaultContractResolver that overwrites its CreateDictionaryContract method.
For the contract that applies to the Dictionary<byte, object> type, set its ItemConverter to your converter (remove the this.conversion shenanigans from it).

protected override JsonDictionaryContract CreateDictionaryContract(Type objectType)
{
    var contract = base.CreateDictionaryContract(objectType);

    if (objectType == typeof(Dictionary<byte, object>))
    {
        contract.ItemConverter = new ClosedParametersConverter();
    }

    return contract;
}

Assign the custom contract resolver to the ContractResolver property of your JsonSettings and also remove the Converters assignment from the JsonSettings.

Note that by applying the converter as the items converter of the contract, the converter's CanConvert method will not be called anymore and the converter will be used for any value in the dictionary regardless of its actual value type.

[elgonzo]

FYI: I made a quick'n'dirty dotnetfiddle demo with the custom contract resolver i mentioned based on your code example: https://dotnetfiddle.net/VBK7n0

[shvez]

@elgonzo got it working with using your proposal. Thank you very much. but had to add case for Hashtable like this:

if (objectType == typeof(Hashtable))
        {
            contract.ItemConverter = new ClosedParametersConverter();
        }

[shvez]

@elgonzo unfortunately proposed solution did not work well in all the cases, so, I had to override array contract creation and object contract creation