runtime: System.Text.Json does not support TypeConverters

Describe the bug

I was used to have custom TypeConverter to convert complex types from/to strings It was working when the serializator was Newtonsoft, now that you moved to internal dotnet serializator it doesn’t work anymore

To Reproduce

Create a controller that returns a class with a complex type where you implemented a TypeConverter for (and registered using TypeDescriptor.AddAttributes ) Look the result of the controller and you’ll see that the output will not be serialized according to the TypeConverter but will be serialized fully as a complex object

We will close this issue if:

  • the behavior will be consistent with newtonsoft serializator behavior

Further technical details

  • tried on ASP.NET Core version 3.1

Note that using the package Microsoft.AspNetCore.Mvc.NewtonsoftJson and doing services.AddControllers().AddNewtonsoftJson() (so basically restoring newtonsoft serializator) fixes this problem (basically bypass internal json serializator)

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 5
  • Comments: 17 (6 by maintainers)

Most upvoted comments

Here’s a intermediate solution to make any type having a [TypeConverter] attribute serialize to/from a string value. Works for me, but might have issues with other cases. Use at your own risk…

class UniversalInvariantTypeConverterJsonConverter : JsonConverterFactory
{
    public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
    {
        var typeConverter = TypeDescriptor.GetConverter(typeToConvert);
        var jsonConverter = (JsonConverter?) Activator.CreateInstance(typeof(TypeConverterJsonConverter<>).MakeGenericType(
                new Type[] {typeToConvert}),
            new object[] {typeConverter, options});
        return jsonConverter;
    }

    public override bool CanConvert(Type typeToConvert) =>
        typeToConvert.GetCustomAttribute<TypeConverterAttribute>() != null;

    private class TypeConverterJsonConverter<T> : JsonConverter<T>
    {
        private readonly TypeConverter _typeConverter;

        public TypeConverterJsonConverter(TypeConverter tc, JsonSerializerOptions options) =>
            _typeConverter = tc;

        public override bool CanConvert(Type typeToConvert) =>
            _typeConverter.CanConvertFrom(typeof(string)) || _typeConverter.CanConvertTo(typeof(string));

        public override T? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
            (T?) _typeConverter.ConvertFromInvariantString(reader.GetString());

        public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
        {
            if (value != null && _typeConverter.ConvertToInvariantString(value) is { } x)
            {
                writer.WriteStringValue(x);
            }
            else
            {
                writer.WriteNullValue();
            }
        }
    }
}

Use like this:

JsonSerializer.Serialize(myItem, new JsonSerializerOptions(){Converters = { new UniversalInvariantTypeConverterJsonConverter()}})

@layomia see responses inline

We’ve added support for more primitives as dictionary keys in 5.0 (#38056), including DateTime. To support this feature, we added new internal methods on JsonConverter<T> to handle reading/writing non-string types from/to JSON property names. This mechanism could be exposed and could also handle complex types like POCOs as dictionary keys. cc @Jozkee

Thank you for the update. That is good news and definitely a step in the right direction. I did verify it in preview8 of the v5 SDK.

The lookup and invocation of these TypeConverter instances is likely non-trivial wrt performance and would likely work against performance goals of System.Text.Json. It would also add new internal code to the library to support a legacy model.

I put together a sample repository that benchmarks the type converter operations and it was running anywhere from 64ms to 45ms inside my VM on a Surface Pro 5. As far as the performance goals of System.Text.Json let’s just say we have to agree to disagree here… System.Text.Json performs way too close to Json.NET and not anywhere close to UTF8Json for that to continue to be the mantra if we’re being honest. Outperforming Json.NET should definitely be the goal and you’ve hit that. Unfortunately feature parity still makes me prefer to use Json.NET over System.Text.Json 100% of the time because the gap in performance is not so substantial to close the feature parity gap. In .NET 5 I want to cut over to web assembly but that exclusively uses System.Text.Json so now the gaps are starting to become glaring.

Can you tell me more about why TypeConverters need to be used here, e.g. in your NodaValues example? Which types are not serializing correctly in System.Text.Json without TypeConverters? Can System.Text.Json’s custom converters be used instead? A full repro app with all the types being (de)serialized along with the type converters would be useful here.

TypeConverters don’t need to be used other than they have been around for ages and work wonderfully as evidenced by how fluidly Json.NET handles custom structs vs System.Text.Json. As far as which types don’t work that would be all of them here is the difference with the LocalTime struct…

Json.NET which uses the TypeConverter to string

"date":"2020-08-27"

System.Text.Json which fumbles across all the properties on the struct

    "date": {
        "calendar": {
            "id": "ISO",
            "name": "ISO",
            "minYear": -9998,
            "maxYear": 9999,
            "eras": [
                {
                    "name": "BCE"
                },
                {
                    "name": "CE"
                }
            ]
        },
        "year": 2020,
        "month": 8,
        "day": 27,
        "dayOfWeek": 4,
        "yearOfEra": 2020,
        "era": {
            "name": "CE"
        },
        "dayOfYear": 240
    }

I have added a JsonConverter for LocalDate that fixes the issue to and from the wire when used explicitly as a scalar property of an object. Unfortunately even with the TypeConverter registered it still blows up when you try to produce a dictionary with LocalDate as a key so I added a DictionaryKeyTypeConverter so yes all of that does work.

The problem is what happens when you make IDictionary, IReadOnlyDictionary, ReadOnlyDictionary, IImmutableDictionary, or ImmutableDictionary parameters? I get you don’t want to use TypeConverters because they are viewed as legacy but in the case of only using them explicitly to and from strings to solve issues like dictionary keys or if the object is a struct and is missing a JsonConverter seems like a no brainer to me.

Another thing when serializing and deserializing dictionaries that have a key you should also check if a type converter to string exists for the key and if so use it.

Currently it blows up if you return a Dictionary<DateTime, string> even though DateTIme has a TypeConverter to string that could be used.

Per feedback, adding built-in support for TypeConverters is not in our roadmap. It can be easily worked around by authoring a custom converter. We heartfully encourage battle tested implementations finding their way in NuGet packages, which seems to already be the case.

@Jozkee thank you for the expedient follow up. So how do I plumb the JsonConverter into getting picked up as a key converter?

I get you don’t want to use TypeConverters because they are viewed as legacy

@buvinghausen That’s not the reason why we didn’t took the approach of relying on TypeConverters for dictionary keys. The main concern is that TypeConverters APIs onle receive and return object, which involves a lot of boxing when dealing with value types (structs) and boxing involves a lot of performance penalty and memory allocation:

Please see the spec for extending support of dictionary keys. https://github.com/dotnet/runtime/blob/a5d96c0e280b56412d4614848f5ee3b1e0d7f216/src/libraries/System.Text.Json/docs/KeyConverter_spec.md#typeconverter

The results are form very early steps in the feature development but you can notice that the TypeConverter approach for Guid was 2x slower than using the “KeyConverter” approach that was using generics:

KeyConverter:

Type Method Mean Error StdDev Allocated
WriteDictionary<Dictionary<Guid, Int32>> SerializeToUtf8Bytes 9.874 us 0.1383 us 0.1226 us 5 KB

TypeConverter:

Type Method Mean Error StdDev Allocated
WriteDictionary<Dictionary<Guid, Int32>> SerializeToUtf8Bytes 19.067 us 0.6886 us 0.7930 us 17.5 KB