[System.Text.Json] JsonConverter lacks ability to specify converter for items in a collection

[RamjotSingh]

JsonConverter allows specifying the conveter to be used for a specific property, however if a collection is in use and you want to use the converter to serialize a items in collection it doesn't work.

Example

public enum Day
{
   Sunday,
   Monday,
}

public class Model
{
    [JsonConverter(typeof(JsonStringEnumConverter))]
    public IEnumerable<Day> Days { get; set; }
}

The above code will throw. The only way to work around is to write a custom converter which understands collections and uses a different converter for each item in collection.

I propose a new attribute which allows the caller to specify converter to use for items in collection should be added. Something like

[JsonCollectionItemConverter(typeof(JsonStringEnumConverter))]

Newtonsoft.Json allows this by allowing specifying the type in JsonProperty attribute. That option has it's own shortcomings as well. I believe the above option will solve that too.

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

---

JsonConverter allows specifying the conveter to be used for a specific property, however if a collection is in use and you want to use the converter to serialize a items in collection it doesn't work.

Example

public enum Day
{
   Sunday,
   Monday,
}

public class Model
{
    [JsonConverter(typeof(JsonStringEnumConverter))]
    public IEnumerable<Day> Days { get; set; }
}

The above code will throw. The only way to work around is to write a custom converter which understands collections and uses a different converter for each item in collection.

I propose a new attribute which allows the caller to specify converter to use for items in collection should be added. Something like

[JsonCollectionItemConverter(typeof(JsonStringEnumConverter))]

Newtonsoft.Json allows this by allowing specifying the type in JsonProperty attribute. That option has it's own shortcomings as well. I believe the above option will solve that too.

Author:	RamjotSingh
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

Playing devil's advocate, is there anything in your scenario that prevents you from doing

[JsonConverter(typeof(JsonStringEnumConverter))]
public enum Day
{
   Sunday,
   Monday,
}

or registering the converter for the enum via JsonSerializerOptions?

[RamjotSingh]

There are a lot of reasons to not do this

1. You want per reference or per enum serialization behavior
2. From reading code perspective, it is easier to understand things if the converter is directly on the property itself since rest of the info (JsonPropertyName).

I don't think 3 ways of specifying converter are not replacement for each other.

[RamjotSingh]

If someone else hits this same problem. Here is a workaround. Its not the sanest solution hence I would like the library to support it out of the box.

     /// <summary>
    /// Json collection converter.
    /// </summary>
    /// <typeparam name="TDatatype">Type of item to convert.</typeparam>
    /// <typeparam name="TConverterType">Converter to use for individual items.</typeparam>
    public class JsonCollectionItemConverter<TDatatype, TConverterType> : JsonConverter<IEnumerable<TDatatype>>
        where TConverterType : JsonConverter
    {
        /// <summary>
        /// Reads a json string and deserializes it into an object.
        /// </summary>
        /// <param name="reader">Json reader.</param>
        /// <param name="typeToConvert">Type to convert.</param>
        /// <param name="options">Serializer options.</param>
        /// <returns>Created object.</returns>
        public override IEnumerable<TDatatype> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            if (reader.TokenType == JsonTokenType.Null)
            {
                return default(IEnumerable<TDatatype>);
            }

            JsonSerializerOptions jsonSerializerOptions = new JsonSerializerOptions(options);
            jsonSerializerOptions.Converters.Clear();
            jsonSerializerOptions.Converters.Add(Activator.CreateInstance<TConverterType>());

            List<TDatatype> returnValue = new List<TDatatype>();

            while (reader.TokenType != JsonTokenType.EndArray)
            {
                if (reader.TokenType != JsonTokenType.StartArray)
                {
                    returnValue.Add((TDatatype)JsonSerializer.Deserialize(ref reader, typeof(TDatatype), jsonSerializerOptions));
                }

                reader.Read();
            }

            return returnValue;
        }

        /// <summary>
        /// Writes a json string.
        /// </summary>
        /// <param name="writer">Json writer.</param>
        /// <param name="value">Value to write.</param>
        /// <param name="options">Serializer options.</param>
        public override void Write(Utf8JsonWriter writer, IEnumerable<TDatatype> value, JsonSerializerOptions options)
        {
            if (value == null)
            {
                writer.WriteNullValue();
                return;
            }

            JsonSerializerOptions jsonSerializerOptions = new JsonSerializerOptions(options);
            jsonSerializerOptions.Converters.Clear();
            jsonSerializerOptions.Converters.Add(Activator.CreateInstance<TConverterType>());

            writer.WriteStartArray();

            foreach (TDatatype data in value)
            {
                JsonSerializer.Serialize(writer, data, jsonSerializerOptions);
            }

            writer.WriteEndArray();
        }
    }

[ericstj]

@steveharter does this get any easier with #36785?

[eiriktsarpalis]

@RamjotSingh would the solution proposed in #63791 address your use case? I was thinking it might be possible to do something like this:

public class Model
{
    [JsonConverter(typeof(JsonCollectionConverter), typeof(JsonStringEnumConverter))]
    public IEnumerable<Day> Days { get; set; }
}

It would use the collection converter that accepts the type of the element converter used.

[eiriktsarpalis]

FWIW it might be a breaking change if we do decide to have JsonConverterAttribute map to element converters. Consider the following example:

public class MyClass : IEnumerable<MyClass>
{
     public MyClass Current => this;
     public bool MoveNext() => true;
}

public class Model
{
    [JsonConverter(typeof(MyClassConverter))] // Not clear whether it should map to the IEnumerable 
                                              // converter or use the converter for the property value directly.
    public IEnumerable<MyClass> MyClass { get; set; }
}

[steveharter]

This was previously discussed and supported by Newtsonsoft's JsonPropertyAttribute.ItemConverterType.

The element converter is there today of course, just not exposed.

Although adding an ItemConverter mechanism will probably work for all resonable scenarios, consider the encapsulation done today by something like a property public List<List<MyClass>> {get;} - we'd need 2 levels of indirection to specify a converter for MyClass. Also consider indirection with generic parameters like public Dictionary<string, MyClass> {get;} as well.

[eiriktsarpalis]

This was previously discussed and supported by Newtsonsoft's JsonPropertyAttribute.ItemConverterType.

Having a dedicated property for the element type is something that would resolve the ambiguity in #54189 (comment). I think it's reasonable that such a feature should only support basic generics patterns, and not concern itself with nested collections.

[danmoseley]

matching partner request tracked here: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1486854

[eiriktsarpalis]

Given that convention already lets users specify custom converter types for element types in nullable properties:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.Converters.cs

Lines 359 to 374 in 008f128

	if (!converter.CanConvert(typeToConvert))
	{
	Type? underlyingType = Nullable.GetUnderlyingType(typeToConvert);
	if (underlyingType != null && converter.CanConvert(underlyingType))
	{
	if (converter is JsonConverterFactory converterFactory)
	{
	converter = converterFactory.GetConverterInternal(underlyingType, this);
	}
	// Allow nullable handling to forward to the underlying type's converter.
	return NullableConverterFactory.CreateValueConverter(underlyingType, converter);
	}
	ThrowHelper.ThrowInvalidOperationException_SerializationConverterOnAttributeNotCompatible(classTypeAttributeIsOn, memberInfo, typeToConvert);
	}

I'm thinking we could extend this convention to collection types without exposing a dedicated property for that purpose.

[eiriktsarpalis]

As with the related #63791, we won't be able to look at this in time for .NET 7, moving to 8.0.0

[eiriktsarpalis]

Moving to Future as we won't be able to work on this for 8.0

[softlion]

You should work on making usage easier. Not on delaying basic things like that for 10 years.

[gpuchtel]

Bump. Any movement on this? Gotta say, I'm surprised this (usage) feature has not been addressed.

[gpuchtel-grubbrr]

@eiriktsarpalis I don't see what the debate is about, NewtonSoft has clearly demonstrated the definition and use of the JsonPropertyAttribute: ItemConverterType. It is clearly needed, so I don't understand the confusion and resistance.

[eiriktsarpalis]

@gpuchtel it's not an issue of debate, we acknowledge that this is an issue that needs to be addressed eventually but we just haven't been able to prioritize work for it so far.

[gpuchtel-grubbrr]

Okay, thanks for the reply, but this seems so fundamental. Eventually means never. Can you give us a workaround? Anything I can get from 'DictionaryOfTKeyTValueConverter'?

[eiriktsarpalis]

The only workaround available today is applying the custom converter directly on the element type definition itself.

[gpuchtel-grubbrr]

I did that, it does not work. Also, it's the Cosmos client that is doing the (de)serialization. I will provide the code-snippet (later) tonight

[gpuchtel]

@eiriktsarpalis after some discussion on 'stackoverflow' [https://stackoverflow.com/questions/78216715/how-can-i-apply-a-custom-system-text-json-jsonconverter-to-the-values-of-a-concu] I've concluded that the (proposed) solutions are not worth the effort and I am fortunate that my custom class is easily convertible to a 'double'. So, I was able to define my value type as a 'double' and with conversion operators. But, I guess I will have to wait if I need to serialize a complex type with System.Text.Json. As a developer, I fully understand priorities, but I urge you to reconsider the importance of this feature. Thanks again, for your time and consideration.

[eiriktsarpalis]

Have you tried either 1) registering your custom Saturation converter with JsonSerializerOptions.Converters or 2) adding a JsonCoverterAttribute annotation to your Saturation class?

[RamjotSingh]

Here is the code that works for us (we built it as a workaround). Can you check if this is working for you?

namespace Microsoft.Json.CustomJsonConverters
{
    using System;
    using System.Collections.Generic;
    using System.Text.Json;
    using System.Text.Json.Serialization;

    /// <summary>
    /// Json collection converter.
    /// </summary>
    /// <typeparam name="TDatatype">Type of item to convert.</typeparam>
    /// <typeparam name="TConverterType">Converter to use for individual items.</typeparam>
    public class JsonCollectionItemConverter<TDatatype, TConverterType> : JsonConverter<IEnumerable<TDatatype>>
        where TConverterType : JsonConverter
    {
        /// <summary>
        /// Reads a json string and deserializes it into an object.
        /// </summary>
        /// <param name="reader">Json reader.</param>
        /// <param name="typeToConvert">Type to convert.</param>
        /// <param name="options">Serializer options.</param>
        /// <returns>Created object.</returns>
        public override IEnumerable<TDatatype> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            if (reader.TokenType == JsonTokenType.Null)
            {
                return default(IEnumerable<TDatatype>);
            }

            JsonSerializerOptions jsonSerializerOptions = new JsonSerializerOptions(options);
            jsonSerializerOptions.Converters.Clear();
            jsonSerializerOptions.Converters.Add(Activator.CreateInstance<TConverterType>());

            List<TDatatype> returnValue = new List<TDatatype>();

            while (reader.TokenType != JsonTokenType.EndArray)
            {
                if (reader.TokenType != JsonTokenType.StartArray)
                {
                    returnValue.Add((TDatatype)JsonSerializer.Deserialize(ref reader, typeof(TDatatype), jsonSerializerOptions));
                }

                reader.Read();
            }

            return returnValue;
        }

        /// <summary>
        /// Writes a json string.
        /// </summary>
        /// <param name="writer">Json writer.</param>
        /// <param name="value">Value to write.</param>
        /// <param name="options">Serializer options.</param>
        public override void Write(Utf8JsonWriter writer, IEnumerable<TDatatype> value, JsonSerializerOptions options)
        {
            if (value == null)
            {
                writer.WriteNullValue();
                return;
            }

            JsonSerializerOptions jsonSerializerOptions = new JsonSerializerOptions(options);
            jsonSerializerOptions.Converters.Clear();
            jsonSerializerOptions.Converters.Add(Activator.CreateInstance<TConverterType>());

            writer.WriteStartArray();

            foreach (TDatatype data in value)
            {
                JsonSerializer.Serialize(writer, data, jsonSerializerOptions);
            }

            writer.WriteEndArray();
        }
    }
}

How to use?

[JsonPropertyName("changeTypes")]
[System.Text.Json.Serialization.JsonConverter(typeof(JsonCollectionItemConverter<ResourceChangeType, JsonStringEnumConverter>))]
public IEnumerable<ResourceChangeType> ChangeTypes { get; set; }

First is the data type, second is the Converter to use for each item.

[gpuchtel]

@eiriktsarpalis "Have you tried either 1) registering your custom Saturation converter with JsonSerializerOptions.Converters or 2) adding a JsonCoverterAttribute annotation to your Saturation class?"

Answer(s), yes to #2 (did not work), no to #1 as I don't have control over the options, as its the Cosmos Client doing the (de)serialization

**** UPDATE ****

Yes, adding a JsonConverterAttribute to the class annotation worked! Sorry, I misread it at first, as I was thinking of the Dictionary annotation.

[gpuchtel]

@RamjotSingh Thanks, I'll give a try...

[eiriktsarpalis]

yes to #2 (did not work)

Can you share a repro?

[gpuchtel]

@eiriktsarpalis Yes, it did work! I gave an 'update' above. Thanks for sticking with this. I think I'm good. Actually, this works better than the NewtonSoft solution, because it places the conversion (attribute) declaration where the Type is declared, rather than where it is used.

[gpuchtel]

After much ado, Eirik Tsarpalis (Microsoft) had the answer all along; I wasn't reading it right. Adding a JsonCoverterAttribute annotation to my custom class (Saturation) works! This is better than annotating the point of usage via Newtsonsoft's JsonPropertyAttribute.ItemConverterType—it places the declaration with the class. Thank you, Eirik!

[rcdailey]

Is there information I can read to understand more about what "Saturation" classes are?

[gpuchtel]

@rcdailey The 'Saturation' class is a custom class in my solution. I copied the code verbatim as the type of class is irrelevant to the discussion.

[rcdailey]

It's relevant insofar as it is part of a proposed solution or workaround, so again I'd appreciate an answer to my question as I feel it is important information for the complete understanding of the solutions discussed here.

[eiriktsarpalis]

@rcdailey the workaround should look something like this:

[JsonConverter(typeof(MyCustomConverter))]
public class MyPoco { }

Then serializing instances of, say, List<MyPoco> will use MyCustomConverter for the element types.

[rcdailey]

Thanks Eirik. So the term "Saturation" was actually referring to a custom JSON converter class? If not, what is that referring to? It's not a term I've heard of in the context of System.Text.Json.

[eiriktsarpalis]

That's right, it's the sample class referenced in the linked SO discussion.