Json.Net serialization gives wrong property to custom converter

Question

I have this model that I want to serialize and deserialize using Json.Net:

public struct RangeOrValue
{
    public int Value { get; }
    public int Min { get; }
    public int Max { get; }
    public bool IsRange { get; }

    public RangeOrValue(int min, int max)
    {
        Min = min;
        Max = max;
        IsRange = true;
        Value = 0;
    }

    public RangeOrValue(int value)
    {
        Min = 0;
        Max = 0;
        Value = value;
        IsRange = false;
    }
}

I have a special requirement for serialization. If the first constructor is used, then the value should be serialized as { "Min": <min>, "Max": <max> }. But if the second constructor is used, then value should be serialized as <value>.

For example new RangeOrValue(0, 10) needs to be serialized as { "Min": 0, "Max": 10 } and new RangeOrValue(10) needs to be serialized as 10.

I wrote this custom converter to do this task:

public class RangeOrValueConverter : JsonConverter<RangeOrValue>
{
    public override void WriteJson(JsonWriter writer, RangeOrValue value, JsonSerializer serializer)
    {
        if (value.IsRange)
        {
            // Range values are stored as objects
            writer.WriteStartObject();
            writer.WritePropertyName("Min");
            writer.WriteValue(value.Min);
            writer.WritePropertyName("Max");
            writer.WriteValue(value.Max);
            writer.WriteEndObject();
        }
        else
        {
            writer.WriteValue(value.Value);
        }
    }

    public override RangeOrValue ReadJson(JsonReader reader, Type objectType, RangeOrValue existingValue, bool hasExistingValue, JsonSerializer serializer)
    {
        reader.Read();
        // If the type is range, then first token should be property name ("Min" property)
        if (reader.TokenType == JsonToken.PropertyName)
        {
            // Read min value
            int min = reader.ReadAsInt32() ?? 0;
            // Read next property name
            reader.Read(); 
            // Read max value
            int max = reader.ReadAsInt32() ?? 0;
            // Read object end
            reader.Read();
            return new RangeOrValue(min, max);
        }

        // Read simple int
        return new RangeOrValue(Convert.ToInt32(reader.Value));
    }
}

To test the functionality, I wrote this simple test:

[TestFixture]
public class RangeOrValueConverterTest
{
    public class Model
    {
        public string Property1 { get; set; }
        public RangeOrValue Value { get; set; }
        public string Property2 { get; set; }
        public RangeOrValue[] Values { get; set; }
        public string Property3 { get; set; }
    }

    [Test]
    public void Serialization_Value()
    {
        var model = new Model
        {
            Value = new RangeOrValue(10),
            Values = new[] {new RangeOrValue(30), new RangeOrValue(40), new RangeOrValue(50),},
            Property1 = "P1",
            Property2 = "P2",
            Property3 = "P3"
        };

        string json = JsonConvert.SerializeObject(model, new RangeOrValueConverter());

        var deserializedModel = JsonConvert.DeserializeObject<Model>(json, new RangeOrValueConverter());

        Assert.AreEqual(model, deserializedModel);
    }
}

When I run the test, Object serializes successfully. But when it tries to deserialize it back, I receive this error:

Newtonsoft.Json.JsonReaderException : Could not convert string to integer: P2. Path 'Property2', line 1, position 46.

The stack trace leads to line int min = reader.ReadAsInt32() ?? 0;.

I think I'm doing something wrong in converter that causes Json.Net to provide wrong values to the converter. But I can't quite figure it out. Any ideas?

Answer 1

Your basic problem is that, at the beginning of ReadJson(), you unconditionally call Read() to advance the reader past the current token:

public override RangeOrValue ReadJson(JsonReader reader, Type objectType, RangeOrValue existingValue, bool hasExistingValue, JsonSerializer serializer)
{
    reader.Read(); 

However, if the current token is an integer corresponding to a RangeOrValue with a single value, then you have just skipped past that value, leaving the reader positioned on whatever comes next. Instead you need to process the current value when that value is of type JsonToken.Integer.

That being said, there are several other possible issues with your converter, mostly related to the fact that you assume that the incoming JSON is in a particular format, rather than validating that fact:

• According to the JSON standard an object is an unordered set of name/value pairs but ReadJson() assumes a specific property order.

• ReadJson() doesn't skip past or error on unknown properties.

• ReadJson() doesn't error on truncated files.

• ReadJson() doesn't error on unexpected token types (say, an array instead of an object or integer).

• If the JSON file contains comments (which are not included in the JSON standard but are supported by Json.NET) then ReadJson() will not handle this.

• The converter doesn't handle Nullable<RangeOrValue> members.

  Note that if you inherit from JsonConverter<T>, then you will have to write separate converters for T and Nullable<T>. Thus, for structs, I think it's easier to inherit from the base class JsonConverter.

A JsonConverter that handles these issues would look something like the following:

public class RangeOrValueConverter : JsonConverter
{
    const string MinName = "Min";
    const string MaxName = "Max";

    public override bool CanConvert(Type objectType)
    {
        return objectType == typeof(RangeOrValue) || Nullable.GetUnderlyingType(objectType) == typeof(RangeOrValue);
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        var range = (RangeOrValue)value;
        if (range.IsRange)
        {
            // Range values are stored as objects
            writer.WriteStartObject();
            writer.WritePropertyName(MinName);
            writer.WriteValue(range.Min);
            writer.WritePropertyName(MaxName);
            writer.WriteValue(range.Max);
            writer.WriteEndObject();
        }
        else
        {
            writer.WriteValue(range.Value);
        }
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        switch (reader.MoveToContent().TokenType)
        {
            case JsonToken.Null:
                // nullable RangeOrValue; return null.
                return null;  

            case JsonToken.Integer:
                return new RangeOrValue(reader.ValueAsInt32());

            case JsonToken.StartObject:
                int? min = null;
                int? max = null;
                var done = false;
                while (!done)
                {
                    // Read the next token skipping comments if any
                    switch (reader.ReadToContentAndAssert().TokenType)
                    {
                        case JsonToken.PropertyName:
                            var name = (string)reader.Value;
                            if (name.Equals(MinName, StringComparison.OrdinalIgnoreCase))
                                // ReadAsInt32() reads the NEXT token as an Int32, thus advancing past the property name.
                                min = reader.ReadAsInt32();
                            else if (name.Equals(MaxName, StringComparison.OrdinalIgnoreCase))
                                max = reader.ReadAsInt32();
                            else
                                // Unknown property name.  Skip past it and its value.
                                reader.ReadToContentAndAssert().Skip();
                            break;

                        case JsonToken.EndObject:
                            done = true;
                            break;

                        default:
                            throw new JsonSerializationException(string.Format("Invalid token type {0} at path {1}", reader.TokenType, reader.Path));
                    }
                }
                if (max != null && min != null)
                    return new RangeOrValue(min.Value, max.Value);
                throw new JsonSerializationException(string.Format("Missing min or max at path {0}", reader.Path));

            default:
                throw new JsonSerializationException(string.Format("Invalid token type {0} at path {1}", reader.TokenType, reader.Path));
        }
    }
}

Using the extension methods:

public static partial class JsonExtensions
{
    public static int ValueAsInt32(this JsonReader reader)
    {
        if (reader == null)
            throw new ArgumentNullException();
        if (reader.TokenType != JsonToken.Integer)
            throw new JsonSerializationException("Value is not Int32");
        try
        {
            return Convert.ToInt32(reader.Value, NumberFormatInfo.InvariantInfo);
        }
        catch (Exception ex)
        {
            // Wrap the system exception in a serialization exception.
            throw new JsonSerializationException(string.Format("Invalid integer value {0}", reader.Value), ex);
        }
    }

    public static JsonReader ReadToContentAndAssert(this JsonReader reader)
    {
        if (reader == null)
            throw new ArgumentNullException();
        while (reader.Read())
        {
            if (reader.TokenType != JsonToken.Comment)
                return reader;
        }
        throw new JsonReaderException(string.Format("Unexpected end at path {0}", reader.Path));
    }

    public static JsonReader MoveToContent(this JsonReader reader)
    {
        if (reader == null)
            throw new ArgumentNullException();
        if (reader.TokenType == JsonToken.None)
            if (!reader.Read())
                return reader;
        while (reader.TokenType == JsonToken.Comment && reader.Read())
            ;
        return reader;
    }
}

However, if you're willing to pay a slight performance penalty, the converter can be simplified by serializing and deserializing a DTO as shown below, which uses the same extension method class:

public class RangeOrValueConverter : JsonConverter
{
    class RangeDTO
    {
        public int Min, Max;
    }

    public override bool CanConvert(Type objectType)
    {
        return objectType == typeof(RangeOrValue) || Nullable.GetUnderlyingType(objectType) == typeof(RangeOrValue);
    }

    public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
    {
        var range = (RangeOrValue)value;
        if (range.IsRange)
        {
            var dto = new RangeDTO { Min = range.Min, Max = range.Max };
            serializer.Serialize(writer, dto);
        }
        else
        {
            writer.WriteValue(range.Value);
        }
    }

    public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer)
    {
        switch (reader.MoveToContent().TokenType)
        {
            case JsonToken.Null:
                // nullable RangeOrValue; return null.
                return null;

            case JsonToken.Integer:
                return new RangeOrValue(reader.ValueAsInt32());

            default:
                var dto = serializer.Deserialize<RangeDTO>(reader);
                return new RangeOrValue(dto.Min, dto.Max);
        }
    }
}

Demo fiddle showing both converters here.