Support for custom converters and OnXXX callbacks

[steveharter]

Primary scenarios for converters:

• To support a custom data type (e.g. a PhoneNumber struct stored as a JSON string). Data types are typically immutable structs that have a constructor taking the input and no property setters.
  • Since a converter instantiates the value or object, the converter has flexibility to call constructors with parameters. The converter may also set public fields or use reflection to set non-public state.
  • Data types can either be JSON primitives (number, string, boolean) or JSON objects.
• To support non-trivial POCO scenarios that have semantics based on state:
  • Property dependencies and ordering. For example, in order to deserialize a given property, it may need state from another property.
  • Adding or removing properties dynamically.
  • Polymorphic deserialization. Today the deserializer doesn't support polymorphism except deserializing into a JsonElement if the corresponding property is object.
• To override built-in converters default (de)serialization.
  • Common cases include DateTime, specific Enums, and supporting JSON strings for integers or floating point types.

Update: currently OnXXX callback are out of scope for 3.0. See the comments section for a workaround using a converter.
OnXXX callbacks are methods that are invoked on the POCO object during (de)serialization. They are specified by adding an attribute to each such method. Scenarios for OnXXX callbacks mostly relate to defaulting and validation scenarios:
- OnDeserializing: initialize getter-only collections before deserialization occurs.
- OnDeserialized: initialize unassigned properties; set calculated properties in preparation for consumption; throw exception if missing or invalid state.
- OnSerializing: assign default values; throw exception if missing or invalid state; set up any temporary variables used for serialization workarounds.
- OnSerialized: clear any temporary variables.

To create a new converter:

1. Create a class that derives from JsonValueConverter<T> which closes the <T> to the type to convert.
2. Override the Read and Write methods.
3. Have the user register the converter by registering through JsonSerializerOptions or by placing [JsonConverter] on a property. Optionally, if the converter is for a custom data type, the data type itself can have a [JsonConverter] which will self-register.

API

// Runtime registration
namespace System.Text.Json
{
   public class JsonSerializationOptions // existing class
   {
. . .
        /// <summary>
        /// The list of custom converters.
        /// </summary>
        /// <remarks>
        /// Once serialization or deserialization occurs, the list cannot be modified.
        /// </remarks>
        public IList<JsonConverter> Converters { get; }

        /// <summary>
        /// Returns the converter for the specified type.
        /// </summary>
        /// <param name="typeToConvert">The type to return a converter for.</param>
        /// <returns>
        /// The first converter that supports the given type, or null if there is no converter.
        /// </returns>
        public JsonConverter GetConverter(Type typeToConvert)
. . .
    }

   public class JsonException : Exception // existing class
   {
. . .
        public JsonException() { } 
        public JsonException(string message) { } 
        public JsonException(string message, System.Exception innerException) { } 
. . . 
    }
}

// Custom converter support
namespace System.Text.Json.Serialization
{
    /// <summary>
    /// When placed on a property or type, specifies the converter type to use.
    /// </summary>
    /// <remarks>
    /// The specified converter type must derive from <see cref="JsonConverter"/>.
    /// When placed on a property, the specified converter will always be used.
    /// When placed on a type, the specified converter will be used unless a compatible converter is added to
    /// <see cref="JsonSerializerOptions.Converters"/> or there is another <see cref="JsonConverterAttribute"/> on a property
    /// of the same type.
    /// </remarks>
    [AttributeUsage(AttributeTargets.Property | AttributeTargets.Class | AttributeTargets.Struct, AllowMultiple = false)]
    public class JsonConverterAttribute : JsonAttribute
    {
        /// <summary>
        /// Initializes a new instance of <see cref="JsonConverterAttribute"/> with the specified converter type.
        /// </summary>
        /// <param name="converterType">The type of the converter.</param>
        public JsonConverterAttribute(Type converterType);

        /// <summary>
        /// Initializes a new instance of <see cref="JsonConverterAttribute"/>.
        /// </summary>
        protected JsonConverterAttribute() { } 

        /// <summary>
        /// The type of the converter.
        /// </summary>
        /// <remarks>
        /// An instance of this type will be created automatically unless <see cref="CreateConverter"/> returns a
        /// non-null value.
        /// </remarks>
        public Type ConverterType { get; }
        
        /// <summary>
        /// If overriden, allows a custom attribute to create the converter in order to pass additional state.
        /// </summary>
        /// <returns>
        /// The custom converter, or null if the serializer should create the custom converter.
        /// </returns>
        protected virtual JsonConverter CreateConverter(Type typeToConvert);
    }

    // Non-generic base class; not derived from directly.
    // TDB: better naming for JsonConverter?: JsonBaseConverter

    /// <summary>
    /// Converts an object or value to or from JSON.
    /// </summary>
    public abstract class JsonConverter
    {
        private internal JsonConverter();

        /// <summary>
        /// Determines whether the type can be converted.
        /// </summary>
        /// <param name="typeToConvert">The type is checked as to whether it can be converted.</param>
        /// <returns>True if the type can be converted, false otherwise.</returns>
        public abstract bool CanConvert(Type typeToConvert);
    }

    // TBD: better naming for JsonConverterFactory?: JsonLateBoundConverter, JsonDynamicConverter, JsonSurrogateConverter?

    /// <summary>
    /// Supports converting several types by using a factory pattern.
    /// </summary>
    /// <remarks>
    /// This is useful for converters supporting generics, such as a converter for <see cref="System.Collections.Generic.List{T}"/>.
    /// </remarks>
    public abstract class JsonConverterFactory : JsonConverter
    {
        /// <summary>
        /// When overidden, constructs a new <see cref="JsonConverterFactory"/> instance.
        /// </summary>
        protected internal JsonConverterFactory();

        /// <summary>
        /// Create a converter for the provided <see cref="Type"/>.
        /// </summary>
        /// <param name="typeToConvert">The type to convert.</param>
        /// <returns>
        /// An instance of a <see cref="JsonConverter{T}"/> where T is compatible with <paramref name="typeToConvert"/>.
        /// </returns>        
        protected virtual JsonConverter CreateConverter(Type typeToConvert);
    }

    // JsonConverter<T> is the base class used at run-time for the actual conversions.
    // It is possible to have other base classes like this in the future; for example not exposing reader\writer.

    /// <summary>
    /// Converts an object or value to or from JSON.
    /// </summary>
    /// <typeparam name="T">The <see cref="Type"/> to convert.</typeparam>
    public abstract class JsonConverter<T> : JsonConverter
    {
        /// <summary>
        /// When overidden, constructs a new <see cref="JsonConverter{T}"/> instance.
        /// </summary>
        protected internal JsonConverter();

        /// <summary>
        /// Determines whether the type can be converted.
        /// </summary>
        /// <remarks>
        /// The default implementation is to return True when <paramref name="typeToConvert"/> equals typeof(T).
        /// </remarks>
        /// <param name="typeToConvert"></param>
        /// <returns>True if the type can be converted, False otherwise.</returns>
        public override bool CanConvert(Type typeToConvert);
        
        /// <summary>
        /// Read and convert the JSON to T.
        /// </summary>
        /// <param name="reader">The <see cref="Utf8JsonReader"/> to read from.</param>
        /// <param name="typeToConvert">The <see cref="Type"/> being converted.</param>
        /// <param name="options">The <see cref="JsonSerializerOptions"/> being used.</param>
        /// <returns>The value that was converted.</returns>
        public abstract T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options);

        /// <summary>
        /// Write the value as JSON.
        /// </summary>
        /// <param name="writer">The <see cref="Utf8JsonWriter"/> to write to.</param>
        /// <param name="value">The value to convert.</param>
        /// <param name="options">The <see cref="JsonSerializerOptions"/> being used.</param>
        public abstract void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options);
    }
}

Samples

Custom Enum converter

public enum MyBoolEnum
{
    True = 1,   // JSON is "TRUE"
    False = 2,  // JSON is "FALSE"
    Unknown = 3 // JSON is "?"
}

// A converter used to change Enum value names.
public class MyBoolEnumConverter : JsonConverter<MyBoolEnum>
{
    // CanConvert does not need to be implemented here since we only convert MyBoolEnum.

// A converter for a specific Enum.
private class MyBoolEnumConverter : JsonConverter<MyBoolEnum>
{
    // CanConvert does not need to be implemented here since we only convert MyBoolEnum.

    public override MyBoolEnum Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        string enumValue = reader.GetString();
        if (enumValue == "TRUE")
        {
            return MyBoolEnum.True;
        }

        if (enumValue == "FALSE")
        {
            return MyBoolEnum.False;
        }

        if (enumValue == "?")
        {
            return MyBoolEnum.Unknown;
        }

        throw new JsonException();
    }

    public override void Write(Utf8JsonWriter writer, MyBoolEnum value, JsonSerializerOptions options)
    {
        if (value is MyBoolEnum.True)
        {
            writer.WriteStringValue("TRUE");
        }
        else if (value is MyBoolEnum.False)
        {
            writer.WriteStringValue("FALSE");
        }
        else
        {
            writer.WriteStringValue("?");
        }
    }
}

[Fact]
public static void CustomEnumConverter()
{
    var options = new JsonSerializerOptions();
    options.Converters.Add(new MyBoolEnumConverter());

    {
        MyBoolEnum value = JsonSerializer.Parse<MyBoolEnum>(@"""TRUE""", options);
        Assert.Equal(MyBoolEnum.True, value);
        Assert.Equal(@"""TRUE""", JsonSerializer.ToString(value, options));
    }

    {
        MyBoolEnum value = JsonSerializer.Parse<MyBoolEnum>(@"""FALSE""", options);
        Assert.Equal(MyBoolEnum.False, value);
        Assert.Equal(@"""FALSE""", JsonSerializer.ToString(value, options));
    }

    {
        MyBoolEnum value = JsonSerializer.Parse<MyBoolEnum>(@"""?""", options);
        Assert.Equal(MyBoolEnum.Unknown, value);
        Assert.Equal(@"""?""", JsonSerializer.ToString(value, options));
    }
}

Polymorphic POCO converter

public class Person
{
    public string Name { get; set; }
}

public class Customer : Person
{
    public string OfficeNumber { get; set; }
}

public class Employee : Person
{
    public decimal CreditLimit { get; set; }
}

// A polymorphic POCO converter using a type discriminator.
private class PersonConverter : JsonConverter<Person>
{
    enum TypeDiscriminator
    {
        Customer = 1,
        Employee = 2
    }

    public override bool CanConvert(Type typeToConvert)
    {
        return typeof(Person).IsAssignableFrom(typeToConvert);
    }

    public override Person Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType != JsonTokenType.StartObject)
        {
            throw new JsonException();
        }

        reader.Read();
        if (reader.TokenType != JsonTokenType.PropertyName)
        {
            throw new JsonException();
        }

        string propertyName = reader.GetString();
        if (propertyName != "TypeDiscriminator")
        {
            throw new JsonException();
        }

        reader.Read();
        if (reader.TokenType != JsonTokenType.Number)
        {
            throw new JsonException();
        }

        Person value;
        TypeDiscriminator typeDiscriminator = (TypeDiscriminator)reader.GetInt32();
        switch (typeDiscriminator)
        {
            case TypeDiscriminator.Customer:
                value = new Customer();
                break;

            case TypeDiscriminator.Employee:
                value = new Employee();
                break;

            default:
                throw new JsonException();
        }

        while (reader.Read())
        {
            if (reader.TokenType == JsonTokenType.EndObject)
            {
                return value;
            }

            if (reader.TokenType == JsonTokenType.PropertyName)
            {
                propertyName = reader.GetString();
                reader.Read();
                switch (propertyName)
                {
                    case "CreditLimit":
                        decimal creditLimit = reader.GetDecimal();
                        ((Customer)value).CreditLimit = creditLimit;
                        break;
                    case "OfficeNumber":
                        string officeNumber = reader.GetString();
                        ((Employee)value).OfficeNumber = officeNumber;
                        break;
                    case "Name":
                        string name = reader.GetString();
                        value.Name = name;
                        break;
                }
            }
        }

        throw new JsonException();
    }

    public override void Write(Utf8JsonWriter writer, Person value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();

        if (value is Customer)
        {
            writer.WriteNumber("TypeDiscriminator", (int)TypeDiscriminator.Customer);
            writer.WriteNumber("CreditLimit", ((Customer)value).CreditLimit);
        }
        else if (value is Employee)
        {
            writer.WriteNumber("TypeDiscriminator", (int)TypeDiscriminator.Employee);
            writer.WriteString("OfficeNumber", ((Employee)value).OfficeNumber);
        }

        writer.WriteString("Name", value.Name);

        writer.WriteEndObject();
    }
}

[Fact]
public static void PersonConverterPolymorphic()
{
    const string customerJson = @"{""TypeDiscriminator"":1,""CreditLimit"":100.00,""Name"":""C""}";
    const string employeeJson = @"{""TypeDiscriminator"":2,""OfficeNumber"":""77a"",""Name"":""E""}";

    var options = new JsonSerializerOptions();
    options.Converters.Add(new PersonConverter());

    {
        Person person = JsonSerializer.Parse<Person>(customerJson, options);
        Assert.IsType<Customer>(person);
        Assert.Equal(100, ((Customer)person).CreditLimit);
        Assert.Equal("C", person.Name);

        string json = JsonSerializer.ToString(person, options);
        Assert.Equal(customerJson, json);
    }

    {
        Person person = JsonSerializer.Parse<Person>(employeeJson, options);
        Assert.IsType<Employee>(person);
        Assert.Equal("77a", ((Employee)person).OfficeNumber);
        Assert.Equal("E", person.Name);

        string json = JsonSerializer.ToString(person, options);
        Assert.Equal(employeeJson, json);
    }
}

Passing additional state to converter using an attribute

// A custom data type representing a point where JSON is "XValue,YValue".
public struct Point
{
    public Point(int x, int y)
    {
        X = x;
        Y = y;
    }

    public int X { get;}
    public int Y { get;}
}

// Converter for a custom data type that has additional state (coordinateOffset).
private class PointConverter : JsonConverter<Point>
{
    private int _coordinateOffset;

    public PointConverter() { }

    public PointConverter(int coordinateOffset = 0)
    {
        _coordinateOffset = coordinateOffset;
    }

    public override Point Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType != JsonTokenType.String)
        {
            throw new JsonException();
        }

        string[] stringValues = reader.GetString().Split(',');
        if (stringValues.Length != 2)
        {
            throw new JsonException();
        }

        if (!int.TryParse(stringValues[0], out int x) || !int.TryParse(stringValues[1], out int y))
        {
            throw new JsonException();
        }

        var value = new Point(x + _coordinateOffset, y + _coordinateOffset);
        return value;
    }

    public override void Write(Utf8JsonWriter writer, Point value, JsonSerializerOptions options)
    {
        string stringValue = $"{value.X - _coordinateOffset},{value.Y - _coordinateOffset}";
        writer.WriteStringValue(stringValue);
    }
}


[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public class PointConverterAttribute : JsonConverterAttribute
{
    public PointConverterAttribute(int coordinateOffset = 0) : base()
    {
        CoordinateOffset = coordinateOffset;
    }

    public int CoordinateOffset { get; private set; }

    /// <summary>
    /// If overriden, allows a custom attribute to create the converter in order to pass additional state.
    /// </summary>
    /// <returns>The custom converter, or null if the serializer should create the custom converter.</returns>
    public override JsonConverter CreateConverter(Type typeToConvert)
    {
        return new PointConverter(CoordinateOffset);
    }
}

public class ClassWithPointConverterAttribute
{
    [PointConverter(10)]
    public Point Point1 { get; set; }
}

[Fact]
public static void CustomAttributeExtraInformation()
{
    const string json = @"{""Point1"":""1,2""}";

    ClassWithPointConverterAttribute obj = JsonSerializer.Parse<ClassWithPointConverterAttribute>(json);
    Assert.Equal(11, obj.Point1.X);
    Assert.Equal(12, obj.Point1.Y);

    string jsonSerialized = JsonSerializer.ToString(obj);
    Assert.Equal(@"{""Point1"":""11,12""}", jsonSerialized);
}

List converter with additional state

// A List{T} converter that used CreateConverter().
private class ListConverter : JsonConverterFactory
{
    int _offset;

    public ListConverter(int offset)
    {
        _offset = offset;
    }

    public override bool CanConvert(Type typeToConvert)
    {
        if (!typeToConvert.IsGenericType)
            return false;

        Type generic = typeToConvert.GetGenericTypeDefinition();
        if (generic != typeof(List<>))
            return false;

        Type arg = typeToConvert.GetGenericArguments()[0];
        return arg == typeof(int) ||
            arg == typeof(long);
    }

    protected override JsonConverter CreateConverter(Type type)
    {
        Type elementType = type.GetGenericArguments()[0];

        JsonConverter converter = (JsonConverter)Activator.CreateInstance(
            typeof(ListConverter<>).MakeGenericType(elementType),
            BindingFlags.Instance | BindingFlags.Public,
            binder: null,
            new object[] { _offset },
            culture: null);

        return converter;
    }
}

// Demonstrates List<T>; Adds offset to each integer or long to verify converter ran.
private class ListConverter<T> : JsonConverter<List<T>>
{
    private int _offset;
    public ListConverter(int offset)
    {
        _offset = offset;
    }

    public override List<T> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType != JsonTokenType.StartArray)
        {
            throw new FormatException();
        }

        var value = new List<T>();

        while (reader.Read())
        {
            if (reader.TokenType == JsonTokenType.EndArray)
            {
                return value;
            }

            if (reader.TokenType != JsonTokenType.Number)
            {
                throw new FormatException();
            }

            if (typeof(T) == typeof(int))
            {
                int element = reader.GetInt32();
                IList list = value;
                list.Add(element + _offset);
            }
            else if (typeof(T) == typeof(long))
            {
                long element = reader.GetInt64();
                IList list = value;
                list.Add(element + _offset);
            }
        }

        throw new FormatException();
    }

    public override void Write(Utf8JsonWriter writer, List<T> value, JsonSerializerOptions options)
    {
        writer.WriteStartArray();

        foreach (T item in value)
        {
            if (item is int)
            {
                writer.WriteNumberValue((int)(object)item - _offset);
            }
            else if (item is long)
            {
                writer.WriteNumberValue((long)(object)item - _offset);
            }
            else
            {
                Assert.True(false);
            }
        }

        writer.WriteEndArray();
    }
}

[Fact]
public static void ListConverterOpenGeneric()
{
    const string json = "[1,2,3]";

    var options = new JsonSerializerOptions();
    options.Converters.Add(new ListConverter(10));

    {
        List<int> list = JsonSerializer.Parse<List<int>>(json, options);
        Assert.Equal(11, list[0]);
        Assert.Equal(12, list[1]);
        Assert.Equal(13, list[2]);

        string jsonSerialized = JsonSerializer.ToString(list, options);
        Assert.Equal(json, jsonSerialized);
    }

    {
        List<long> list = JsonSerializer.Parse<List<long>>(json, options);
        Assert.Equal(11, list[0]);
        Assert.Equal(12, list[1]);
        Assert.Equal(13, list[2]);

        string jsonSerialized = JsonSerializer.ToString(list, options);
        Assert.Equal(json, jsonSerialized);
    }
}

Requirements

• Support any type of converter (primitive, POCO, arrays, IEnumerable, etc).
• The reader will be placed on the first token in the TryRead().
• Override internal converters. For example, a new Int32converter could be created to automatically convert a json string to an Int32 (not supported in the default Int32 converter).
• If multiple converters are specified for a type, the first converter that returns true to CanConvert is used.
• Automatically handle null and Nullable<T> (the converter is not called)
• Allow a converter to be passed additional state if created directly or through a [JsonConverter]-derived attribute.
• Allow the Read() and Write() methods to throw exceptions (TBD) caught and re-thrown as JsonException which will be displayed as a nice JsonException ("unable to convert" with JsonPath) with Path set.
• Detect whether the Read() and Write() methods read or wrote too much or too little and if so, throw an exception.
• Prevent the Converters collection on the options class from being changed once (de)serialization has occurred.
• Throw InvalidOperationException if there is more than one converter attribute on a given Type or property.
• For Async Stream scenarios (that support streaming through multiple buffers), a "read-ahead" feature ensures the reader will not run out of buffer within a converter.

Design for open generics (typically collection classes)

Consider a custom List<T> converter:

public class MyListConverter<T> : JsonConverter<List<T>> {...}

The read method:

public override bool TryRead(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options, out List<T> value)

The approach taken here is have a separate type (JsonConverterFactory) with factory method (CreateConverter())

Then the list converter would have both a non-generic MyListConverter class (that derives from JsonConverterFactory) and a generic MyListConverter<T> (that derives from JsonConverter<List<T>>).

Only the non-generic MyListConverter class is added to the options.Converters property. Internally there is another list\dictionary that caches all created converters for a specific T, meaning List<int> is a different entry than List<long>.

Priority of converter registration

Converters can be registered at design-time vs run-time and at a property-level vs. class-level. The basic rules for priority over which is selected:

• Run-time has precedence over design-time
• Property-level has precedence over class-level.
• User-specified has precedence over built-in.

Thus the priority from highest to lowest:

• runtime+property: Future feature: options.Converters.Add(converter, property)
• designtime+property: [JsonConverter] applied to a property.
• runtime+class: options.Converters.Add(converter)
• designtime+class: [JsonConverter] applied to a custom data type or POCO.
• Built-in converters (primitive types, JsonElement, arrays, etc).

[AlexeiScherbakov]

What is a purpose of methods:

public virtual bool CanRead {get;}
public virtual bool CanWrite {get;}

[ahsonkhan]

For reference, this would be a good test case to add once this feature comes online:
https://github.com/dotnet/corefx/issues/37740

[rynowak]

My feedback on this as-written at 7:45 PST 😆

• I'm not totally sold on the need to expose the built-in converters publicly or the need for a .Converters namespace.
• JsonConverter.TypeToConvert should be removed, it's not needed since we went with the CanCreate approach

// Allows a surrogate pattern for strongly-typed scenarios to prevent boxing, such as for Enum.
        public virtual JsonConverter CreateConverter(Type converterType);

I don't recall us talking through this. What does this enable?

public class JsonSerializationOptions
    {
. . .
        IList<JsonConverters> Converters { get; }
. . .
    }

I think what we have here is the best we came up with combining the desire to register JsonConverter and the desire to provide JsonConverter<> to avoid boxing. The need here is the ability to express a collection of converters but allow converters to be generic.

This still bugs me to see JsonConverter without the ability to convert anything.

We should get feedback from a wider group as well.

---

Another approach would be add an interface that expresses an untyped json converter.

public interface IJsonConverter
{
    bool CanConvert(Type type);
    bool TryRead(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options, out object value);
    void Write(Utf8JsonWriter writer, object value, JsonSerializerOptions options);
}

Since this is an interface, JsonConverter<T> will have no problem defining a public API surface with different types, because we can explicitly implement the interface.

This is cleaner for sure. The thing that I think is bothersome is defining an interface that nothing the framework will call. I would imagine the serializer would want to downcast to avoid boxing/casts where possible since the serializer is late-bound.

[steveharter]

@rynowak thanks for the feedback.

I'm not totally sold on the need to expose the built-in converters publicly or the need for a .Converters namespace.

Converters will not be public for 3.0 (maybe never). However it may be desirable to extend or re-use these converters for convenience and performance when (de)serializing non-trivial POCO properties including Enums and collections. So I think it the converters have a good chance of being public at some point pending feedback.

However, since we're not sure if the internal converters will be made public I'll move the base classes to S.T.J.Serialization namespace. If we expose the built-in converters in the future, then those could still belong to S.T.J.Serialization.Converters to reduce the count in the main namespace.

JsonConverter.TypeToConvert should be removed, it's not needed since we went with the CanCreate approach

It is there for convenience and performance, but can be removed. Simple converters that derive from JsonConverter<T> do not need to implement CanConvert if they just convert only type T which will probably be the vast majority of converters, at least all of the data type converters for the framework, with the exception of the Enum converter.

TypeToConvert helps with performance because if there is a direct mapping then that can be implemented via hashtable instead looping through the converters and calling CanConvert for each until a match is found. Currently this is how the framework converters work but again this could be hidden \ removed.

...CreateConverter...
I don't recall us talking through this. What does this enable?

This relates to the discussion around supporting generic converters polymorphically. The current approach works for all cases including the tricky Enum converter which needs to be strongly-typed to avoid boxing but also has generic constraints (where T : struct, Enum). It could also be used for generic collections, such as MyCollection<T>. It could be made internal, but I believe that would limit some scenarios -- I'll do some more prototyping in this area and report back.

... IList Converters { get; } ...
This still bugs me to see JsonConverter without the ability to convert anything.

It allows re-ordering, remove and add which are the intended scenarios. We really didn't discuss making it "easy" to manually call read\write.

Another approach would be add an interface that expresses an untyped json converter... I would imagine the serializer would want to downcast to avoid boxing/casts where possible since the serializer is late-bound.

In most cases having object in a converter implementation is not helpful because the real type (<T>) needs to be instantiated or read from, so yes the framework would downcast any object-based read\write in order to call the "real" read\write. Having an object-based read\write in order to it easier to call is possible (as interface or on the JsonConverter non-generic base class) but since it implies boxing for primitives I would say it is an anti-pattern.

[ahsonkhan]

As part of the JsonConverter scenario to work, there is a need within the JsonSerializer implementation for a stand-alone WritePropertyName method on the Utf8JsonWriter. This method does not need to be public, as long as JsonSerializer/Utf8JsonWriter interop APIs have a way to pass in the property name.

However, we should consider adding the WritePropertyName API as public and simplifying the JsonSerializer/Writer Interop APIs.

So, in conjunction with the APIs proposed in the original post, also consider adding:

namespace System.Text.Json
{
    public sealed partial class Utf8JsonWriter : System.IDisposable
    {
        public void WritePropertyName(JsonEncodedText propertyName) { }
        public void WritePropertyName(System.ReadOnlySpan<byte> utf8PropertyName) { }
        public void WritePropertyName(System.ReadOnlySpan<char> propertyName) { }
        public void WritePropertyName(string propertyName) { }
    }
}

Additionally, we can then simplify the JsonSerializer/Writer interop API (from https://github.com/dotnet/corefx/issues/36714#issuecomment-497282656), and only add the WriteValue APIs:

namespace System.Text.Json
{
    public static partial class JsonSerializer
    {
        public static void WriteValue(Utf8JsonWriter writer, object value, Type type, JsonSerializerOptions options = null) { throw null; }	
        public static void WriteValue<TValue>(Utf8JsonWriter writer, TValue value, JsonSerializerOptions options = null) { throw null; }

        //public static void WriteProperty(Utf8JsonWriter writer, ReadOnlySpan<byte> utf8PropertyName, object value, Type type, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty(Utf8JsonWriter writer, ReadOnlySpan<char> propertyName, object value, Type type, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty(Utf8JsonWriter writer, string propertyName, object value, Type type, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty(Utf8JsonWriter writer, JsonEncodedText propertyName, object value, Type type, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty<TValue>(Utf8JsonWriter writer, ReadOnlySpan<byte> utf8PropertyName, TValue value, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty<TValue>(Utf8JsonWriter writer, ReadOnlySpan<char> propertyName, TValue value, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty<TValue>(Utf8JsonWriter writer, string propertyName, TValue value, JsonSerializerOptions options = null) { throw null; }	
        //public static void WriteProperty<TValue>(Utf8JsonWriter writer, JsonEncodedText propertyName, TValue value, JsonSerializerOptions options = null) { throw null; }
    }
}

The performance benefit of adding the propertyname-value JsonSerializer APIs is not that significant (5-7%, even in the best case scenario of writing a single primitve), and results in a lot more complexity within the implementation (effectively duplicating a lot of the current logic). Therefore, it isn't as useful to add (and we can always consider adding them in the future). Within the Utf8JsonWriter usage (on its own), the benefit of writing the property name/value was more pronounced (10-20%).

[terrajobst]

Notes from today:

Callback attributes

• Why didn't we re-use the existing attributes? We decided to not reuse the existing serialization attributes; this would mean that it's hard for consumers to figure which feature/attribute applies to which serialization framework.

• Why are these attributes? The general belief is that attributes are less invasive in the object model and it mirrors what JSON.NET has been doing. The downside is that the people have to read documentation to know what the method signature is what the expected visibility is. Also, it's valid to apply the attributes to different methods.

• Can we punt the callback attributes entirely? Seems like that.

Converters

• Can we avoid making the attribute polymorphic?
• Can we make it so that people can register a single converter than can switch over a range of types?

We'll meet again tomorrow, attempting to close on these issues.

[steveharter]

Callback attributes

• Can we punt the callback attributes entirely? Seems like that.

Seemingly it is possible to use a converter implementation to do the before\after on read\write instead of OnXXX methods. However, calling back into the serializer for the same type will lead to infinite loop since the same converter will be called. We need to detect this somehow in general (as to not throw StackOverflow and instead throw JsonException or InvalidOperation), but using the converter in this way is not possible until we determine how to prevent re-calling the same converter and instead let the framework serializer.

UPDATE: here are alternate locations for the OnXXX functionality using converters:
OnDeserializing: the POCO constructor or property\field initializers.
OnDeserialized: converter's Read "after" location.
OnSerializing: converter's Write "before" location.
OnSerialized: converter's Write "after" location.

When doing this, the converter cannot pass the same options instance because it will cause an infinite loop. The infinite loop is gracefully detected based on MaxDepthSize and results in InvalidOperationException (not StackOverflowException).

Below we don't pass the options in and instead use the "default" global options instance which will not have any custom converters registered, avoiding the infinite loop. If the default options are not feasible (because of settings) then a new instance of the options will need to be created and any settings applied manually; this will be slow since each new instance caches independently.

public override void Read(ref Utf8JsonReader reader,
        Type type, JsonSerializerOptions options,
        out MyPOCO value)
    {
        // "before" code needs to go into the POCO constructor; code here doesn't have the POCO instance.

        value = JsonSerializer.Read<MyPOCO>(ref reader); // note: "options" not passed in

        // Place "after" code here (e.g. OnDeserialized)
    }

public override void Write(Utf8JsonWriter writer,
        MyPOCO value, JsonSerializerOptions options)
    {
        // Place "before" code here (e.g. OnSerializing)

        JsonSerializer.Write(writer, value); // note: "options" not passed in

        // Place "after" code here (e.g. OnSerialized)
    }

In addition to the workaround above, a future feature that fits with the current design would allow the notifications plus a materializer for an object. The read\write is handled normally by the framework. This works only for JSON objects (not value types) and assumes there is not already a converter for this type. Something like:

    public class ObjectMaterializer<T> : JsonConverter
    {
        public override sealed bool CanConvert(Type typeToConvert);
        public virtual T Deserializing();
        public virtual void Deserialized(T value);
        public virtual void Serializing(T value);
        public virtual void Serialized(T value);
    }

[rynowak]

These are some of my raw notes from design discussions that we had when coming up with the proposal for converters. I'm including these because @ahsonkhan thought they might be useful. I've cut samples and API designs from my notes, because the top post is the authoritative place for that info.

Some of these notes include details or decisions that have already been revised, so consider yourself warned ⚠️

Notes from Ryan

Prebuffering

• We will prebuffer for reads so that user code can be written simply
• We sill buffer for writes so that user code can be written simply

?? open question ?? - Do we validate that a converter read/wrote a single object/value/array. edit: I don't think we made a decision on this, which means that we're probably not going to try and validate this. JSON.NET doesn't attempt to validate this invariant.

Nullable<T> and converters ?? - serialize can decide whether or not to invoke a converter based on null/not-null. If we need to make this configurable we can with a property on the converter, not needed until we get user feedback.

Like JsonDocument - JsonCommentHandling.Allow will throw since it has not supported meaning

Reader/Writer API additions

Seperate reading/writing - a few options

1. JsonSerializer.WriteProperty(writer, "Key", value.Key, options);
2. writer.SerializeProperty("Key", value, options); (extension method in .Serialization namespace)
3. Investigate EVEN HARDER why we can't make this 'just work' with writer.WritePropertyName(...)

We still think dotnet/corefx#3 is viable, but it needs more investigation

Skip() is approved and checked in.

Serializer API additions

Where we have a disagreement between the options used to create the reader/writer and the JsonSerializerOptions that are passed in, we will honor the options on the reader/writer. We need to add this to docs.

Parse() that accepts the reader like (dotnet/CoreFx#36717):

JsonSerializer.Parse<TKey>(reader, options);

Write() that accepts the writer like (dotnet/CoreFx#36714):

writer.WritePropertyName("Value");
JsonSerializer.Write(writer, value.Value, option);

Notes from Steve

• Scenarios to make sure we support:
o Custom enum to string
o F# discriminated unions
 Does not require layering on top of an existing converter.
• User code in converters return false in TryRead to indicate failure
o The serializer will throw JsonException if false is returned
• Should the serializer catch and re-throw exceptions thrown from within a convert read\write method?
o Currently no.
o If so, what exceptions should we catch?
 If we catch JsonException then we can add Path.
• Developer can’t throw nice JsonException themselves since to access to JsonPath
o Need to think more about this, but leave as-is for now
• Change AddConverter(converter) to an IList Converters property.
o Will not contain the default converters in this list.
o Should allow open generics
• Add CanConvert(Type) method to converter base.
o Will use the first converter in list that returns true
o Default implementation is to return true if a match on
• Add tests\support for polymorphic and interface scenarios
o May need to have read\write virtual methods based on object instead of generic
• Support for the 4 callback attributes will be added; reviewed together with converters
o Add scenarios for defaulting and validation
o Called method will likely have no parameters, but we could add options and reader\writer
o If more than one attribute is specified we throw InvalidOperation during warm-up phase.

[terrajobst]

Raw notes from today:

• @KrzysztofCwalina: send some sample code how the Azure SDK would like to convert types?
  • Most types are classes, some are structs.
  • Most of the time, they are in collections?
• Converters are an "escape hatch" to handle serialization/deserialization
  • We need a way to customize the behavior without having to modify the type
  • We also want a design that allows attributes because it's convenient if modifying the type is an option.
  • CanConvert is required to register for converters for types "one cannot easily name", for example, "any enum" or "any F# discriminated union"
  • To avoid performance issues with having to query a long list of converters by calling CanConvert() the serializer caches the answer per type per converter.
• Exception handling
  • Implementers of TryRead shouldn't try/catch and return false.
  • IOW they should return false if the JSON payload is malformed.
  • We're thinking about changing TryRead/TryWrite to Read/Write and the implementer throw JsonException. The question is how this will play with getting more data. It was proposed that the serializer/deserializer makes sure that isn't the case. Also, if the token type is wrong, we currently throw InvalidOperationException. Ideally, the serializer doesn't require converter authors to handle this and instead just let the exception propagate. We can do this by handling InvalidOperationException but this means the serializer might also treat unrelated error cases (which also happens to throw this exception) as malformed JSON.
  • Converters will have to handle some of the JSON options, such as handling of comments, null behavior, and casing.
• We need the feature the binder supports (a factory for converters) but it seems we want to simplify this.

[khellang]

I've just come across a scenario, porting some code away from Newtonsoft.Json, where I really need an equivalent to JsonTextWriter.WritePropertyName on Utf8JsonWriter. I can't see any follow-up on this particular API after https://github.com/dotnet/corefx/issues/36639#issuecomment-500748970. Has this been considered?

[steveharter]

@khellang yes a writer.PropertyName feature is planned. cc @ahsonkhan

[terrajobst]

Video

Notes from toady:

• JsonConverterAttribute: let's make CreateConverter public
• JsonConverter<T>.Read/Write: We shouldn't catch specific types, instead, we should catch exceptions whose Source indicate that it originates from the reader/writer. The reader/writer only marks exception types that it expects the serializer to catch, for example, it would not mark argument exception because those should go unhandled. We should, however, still special case JsonException so that implementers of JSON converters don't have to set the source.
• JsonConverterAttribute we should add protected JsonConverterAttribute() constructor that can be used by derived types.