Use System.Text.Json as the default serializer

[sebastienros]

Fixes #227

[jtkech]

Just for info, in a recent PR I had to work with System.Text.Json, so I planned to work on this ;) But first still need to learn a little more around System.Text.Json when I will have time.

[sebastienros]

One issue will be with polymorphism. For instance if we have a list of content parts, each type needs to be allow-listed with a custom string as a type discriminator. This can be done on the base type so it doesn't work for us. Fortunately there is a code first way. This might mean that we would have to use the existing AddContentPart extension methods for instance. And this would be the same thing for any class that can be serialized in the JSON document. For backward compatibility the discriminator should match the one that Newtonsoft was using.

I have been warned that JsonNode is also not as powerful as JObject so we'll need to understand how.

[jtkech]

Okay, would need more time.

For info I saw that JsonDocument is disposable, for example it rents some data.

[deanmarcussen]

One issue will be with polymorphism. For instance if we have a list of content parts, each type needs to be allow-listed with a custom string as a type discriminator. This can be done on the base type so it doesn't work for us. Fortunately there is a code first way. This might mean that we would have to use the existing AddContentPart extension methods for instance. And this would be the same thing for any class that can be serialized in the JSON document. For backward compatibility the discriminator should match the one that Newtonsoft was using.

I have been warned that JsonNode is also not as powerful as JObject so we'll need to understand how.

ContentParts are actually not a problem, as they are NOT json $type serialized, they are based on registered ContentParts, and the Json.Name matches to that registration.

The most dificult one, is a DeploymentPlan document, which has a List of List<DeploymentStep> where DeploymentStep is an abstract type, and the serializer currently adds the $type property when serializing the document.

Currently this also causes problems if a serialized type is no longer available, as it's been moved / renamed / changed (has happened a lot for us, as we right a lot of custom steps etc), because it throws an exception deep in YesSql/Serialization, and the only fix is to start editing the sql table, to fix the types, which is quite frankly a PITA.

I would suggest, that a registration arrangement for things like DeploymentStep (it's not the only one where this concept is used, it's just the one I'm using as an example, of allowed types, based around the same discriminator, and fallback option choices would be very helpful

[hishamco]

Just for info, in a recent PR I had to work with System.Text.Json, so I planned to work on this ;) But first still need to learn a little more around System.Text.Json when I will have time.

I already planned to do it after an old PR that has been closed #237

@sebastienros if you don't mind I can push some commits here as long as I'm working on related STJ in OC

[sebastienros]

@deanmarcussen good point to content parts. I wanted to make a point and used the wrong example ;) Dean to the rescue. Though I am fine with migrations updating json documents when we do actual breaking changes.

One thing that STJ adds is that we can set the value for the type discriminator value. Like instead of storing the fullname we could just store a number or a simple class name.

Also with DeploymentSteps removed we can implement some custom converters that would handle that, there are samples for that.

It would be a good idea to go over all the documents of a live database to find where $type is used to understand where we need to do custom polymorphism things, or try to remove these?

[jtkech]

Polymorphism is cool, didn't know about this "$type" property, at least we also have Query in that case because it can be a Sql, Lucene or Elastic query, I saw the dedicated converters/attributes in the STJ doc.

I started some tests locally, first problem is the missing JsonObject.Merge(), was not easy to create but found that we can do a partial deserialization to retrieve a JsonElement from a JsonObject which helped me, @kevinchalet opened an issue a while ago for this request.

I will continue but by using net8.0 because they introduced JsonNode.GetValueKind() which may also help, and because they also introduced JsonNode.DeepClone() and DeepEquals() that we are using.

As I can see we are also missing some options, for example when merging arrays.

Hope I will have time to continue ;)

[hishamco]

I started some tests locally, first problem is the missing JsonObject.Merge()

I did some experimental in OC, I created an extensions to merge, the complex part are objects and arrays

introduced JsonNode.DeepClone() and DeepEquals() that we are using

Right

[MikeAlhayek]

@sebastienros rebated and fixed the conflict. Is there more work to be done on this PR or is it something that can be merged?

[MikeAlhayek]

@sebastienros skipped the .NET 7 and onto .NET 8.

[jtkech]

I was not sure that we need something here for BigInteger, but maybe if it fails to convert for example to a long because of the dedicated max/min values.

But we may also need to provide a JsonBigIntegerConverter, for info I found this one here https://github.com/graphql-dotnet/graphql-dotnet/blob/d46bde6d5e30ea1e3a314f40a408f9c86480018d/src/GraphQL.SystemTextJson/JsonConverterBigInteger.cs#L14 in which they define a TryGetBigInteger() that takes care if the reader has more than one segment.

public static bool TryGetBigInteger(ref Utf8JsonReader reader, out BigInteger bi)
{
    var byteSpan = reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
    Span<char> chars = stackalloc char[byteSpan.Length];
    Encoding.UTF8.GetChars(reader.ValueSpan, chars);
    return BigInteger.TryParse(chars, NumberStyles.Integer, NumberFormatInfo.InvariantInfo, out bi);
}

[jtkech]

@sebastienros Just for info

So in OrchardCMS/OrchardCore#14572 I'm using a clone of your DynamicJsonConverter that I named JsonDynamicConverter but that we will no longer need when yours will be there.

This converter fixed issues in the context of GraphQL but using a simple dictionary in place of an ExpandoObject also works, knowing that the dynamic ContentItem.Content is supported differently. So this converter uses the following that I think is sufficient here.

        case JsonTokenType.StartArray:
            var list = new List<object?>();
            ...

        case JsonTokenType.StartObject:
            var dictionary = new Dictionary<string, object?>();
            ...

This converter also checks for decimal just before also checking for BigInteger.

            ...
            if (reader.TryGetDecimal(out var decimalValue))
            {
                return decimalValue;
            }

            if (JConvert.TryGetBigInteger(ref reader, out var bigInteger))
            {
                return bigInteger;
            }

            if (reader.TryGetDouble(out var doubleObject))
            {
                return doubleObject;
            }
            ...

[MikeAlhayek]

Merging this into release/5.0