[BUG] Json serialization behavior is not the same on framework as it is on core/5+

[KieranDevvs]

Describe the bug Deserializing the structs on Framework from valid JSON results in a zero value. Deserializing the structs on Core/NET 5+ results in the correct representation.

The JSON is serialized correctly but it would appear the fields aren't populated correctly when deserializing due to the fact that the properties have private setters. System.Text.Json gained the ability to deserialise properties with private setters in .NET 5.

Potential solutions: 1) Add a [JsonConstructor] to the structs. 2) Package a converter within the library and mark the struct with the converter.

public class TimeOnlyJsonConverter : JsonConverter<TimeOnly>
{
    public override TimeOnly Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        return TimeOnly.Parse(reader.GetString());
    }

    public override void Write(Utf8JsonWriter writer, TimeOnly timeValue, JsonSerializerOptions options)
    {
        writer.WriteStringValue(timeValue.ToString());
    }
}

3) Users have to write their own converters (undesirable as it does not maintain the behavior we have with the original usage of DateOnly and TimeOnly).

To Reproduce

#r "nuget: System.Text.Json, 8.0.3"
#r "nuget: Portable.System.DateTimeOnly, 8.0.0"

using System.Text.Json;

var a = new TimeOnly(1, 2);

var json = JsonSerializer.Serialize(a);

var b = JsonSerializer.Deserialize<TimeOnly>(json);

b.Dump();

.NET 8

.NET Framework 4.8

[OlegRa]

Let me comment on your initial statement a little. You wrote:

The JSON is serialized correctly but it would appear the fields aren't populated correctly when deserializing due to the fact that the properties have private setters. System.Text.Json gained the ability to deserialize properties with private setters in .NET 5.

There is no "correct" serialization for this type (and for the DateOnly too). The System.Text.Json developers made some decisions on how to do it but it's not a part of JSON specification. You can also serialize these types as numbers if it's more appropriate for your use cases.

And BCL versions of DateOnly and TimeOnly types don't have serialization attributes. The BCL doesn't depend on the System.Text.Json library. But the System.Text.Json has a special code for handling these types on some platforms. Of course, the built-in TimeOnlyConverter will handle only the BCL-based version of the TimeOnly type.

I don't particularly appreciate adding dependencies like System.Text.Json into such a tiny library. Everyone who needs "portable" things should understand its limitations. But I agree that this limitation should be more "visible":

• I can add a statement about this "known issue" to the README and encourage users to write converters
• I can provide sample converters as a code in the README so everyone can copy-paste code and use it
• I can create another NuGet package with 2 converters and write about this package in the README

And of course, you can create this new NuGet package with converters and I'll just add a link to it into README.

[OlegRa]

@all-contributors please add @KieranDevvs for ideas

[allcontributors[bot]]

@OlegRa

I've put up a pull request to add @KieranDevvs! :tada:

[KieranDevvs]

@OlegRa Thanks for the feedback. I appreciate not wanting to add a serialization library dependency, and I think having a separate NuGet package for the converters would be great. I don't think this should be left to the user to implement as I see much more benefit and ease of use from standardising the converters to match the .NET core/5+ behaviour.

This would mean users would have to decorate their properties / fields with [JsonConverter(typeof(NameOfConverterType))] or add the converter directly to the JsonSerializerOptions. This is obviously only going to be the case if you're using System.Text.Json and any other serializer will need other workarounds (although I think Newtonsoft.Json doesn't have this problem, and I personally only care about the serializers that are available as part of the BCL). Subsequently this would give a place for users to contribute their own conversion types for other libraries.

[KieranDevvs]

@OlegRa Oh I didn't expect you to create the project AND do the implementation, I was going to contribute but I wont argue about not having to do the work 🥳. Do you have an ETA on the NuGet package release? I can start swapping out my existing converters in place of yours 🙂.

[OlegRa]

If you have an alternative implementation, you can create your own NuGet package. It's always better to have two OSS alternatives than one 🥇

My implementation is based on the System.Text.Json code base. The only interesting thing is making an attribute that will work for old and new frameworks without any ifdefs in the client code. My version fallbacks to the built-in converters in the case of .NET 6.0 and later.

I will publish this NuGet package later this evening (European time) after completing GitHub action for it.

[OlegRa]

@KieranDevvs Finally it's published. Please, test and provide your feedback here or in a separate issue(s).

[osexpert]

I wonder...does it not suffice to use a type converter to\from string? I think both json.net and STJ would then just work without any extra work and no need for separate package?

[KieranDevvs]

I wonder...does it not suffice to use a type converter to\from string? I think both json.net and STJ would then just work without any extra work and no need for separate package?

I don't understand your question. Portable.System.DateTimeOnly.Json does provide a type converter. As mentioned in this issue, its not favorable to let the consumers each individually build their own. It leads to a worse experience as the type cant be used out of the box if you intend to serialize it. It also leads to non standardized serialization so one vendor might build their converter to serialize to .ToString() which would result in something like "YYYY-MM-DD hh:mm:ss" , and another provider might serialize it to a flattened object like so:

{
    "year": 2001,
    "month": 2,
    "day": 19,
    "hour": 17,
    ....
}

And now if these two applications want to interface each other, they need to: A) Share converter types or B) Standardize their converters.

TLDR; I think its better to offer a standard approach out of the box and if you decide to do your own thing, then fine, but you're to blame if it doesn't go well.

[osexpert]

I don't understand your question. Portable.System.DateTimeOnly.Json does provide a type converter

I mean, could it suffice to use a json independent type converter like this that could live in the main package (meaning, in the System.DateTimeOnly package itself):

 [TypeConverter(typeof(DateOnlyIsoTypeConverter))]
struct DateOnly
{
 ...
}

    public class DateOnlyIsoTypeConverter : TypeConverter
    {
        public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
            => sourceType == typeof(string);

        public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object value)
        {
            if (value is string str && DateOnly.TryParseExact(str, "o", CultureInfo.InvariantCulture, DateTimeStyles.RoundtripKind, out var ut))
            { 
                return ut;
            }

            return base.ConvertFrom(context, culture, value);
        }

        public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)
        {
            if (destinationType == typeof(string) && value is DateOnly don)
            {
                return don.ToString("o", CultureInfo.InvariantCulture);
            }

            return base.ConvertTo(context, culture, value, destinationType);
        }
    }

"As mentioned in this issue, its not favorable to let the consumers each individually build their own" I have suggested no such thing

"TLDR; I think its better to offer a standard approach out of the box and if you decide to do your own thing, then fine, but you're to blame if it doesn't go well." Yes, I have not suggested otherwise...

[OlegRa]

@osexpert There are two problems with the TypeConverter approach:

1. The original BCL types don't have this attribute and there is no default type converter(s) for them. Adding these attributes will not affect anyone (I hope), but it will make backported types not fully compatible with the BCL sources.
2. Most importantly - the System.Text.Json library never uses the TypeConverter attribute for controlling the serialization/deserialization process (see this discussion).

The current approach is not ideal but it's better than nothing. Right now it has only one limitation - the source generator goes crazy and tries to use built-in converters for backported types even on unsupported platforms. For this scenario, we'll need a more complex workaround.

[osexpert]

2. Most importantly - the System.Text.Json library never uses the TypeConverter attribute for controlling the serialization/deserialization process (see this discussion).

That is a problem for sure:-) I incorrectly remembered it did. Thanks.

[OlegRa]

@KieranDevvs, I closed it as completed after this helper NuGet package's "official" release. If you have any issues/bugs or some ideas for better documentation, please open another issue(s).