Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for .Net Core 3 Json library #1347

Closed
monsieurleberre opened this issue Nov 19, 2019 · 23 comments
Closed

Support for .Net Core 3 Json library #1347

monsieurleberre opened this issue Nov 19, 2019 · 23 comments

Comments

@monsieurleberre
Copy link

.Net Core 3 comes with its own set of classes to serialise and deserialise JSON, located in the System.Text.Json and System.Text.Json.Serialization.
Previous versions used to rely on Newtonsoft.Json and Newtonsoft.Json.Converters instead.

This also means some attribute names need to be adapted:

        [JsonProperty("id")]
        public Guid Id { get; set; }

becomes

        [JsonPropertyName("id")]
        public Guid Id { get; set; }

Serialisation of enums to string can be done this way

    [JsonConverter(typeof(JsonStringEnumConverter))]
    public enum Color { Red, Green, Blue };

but cannot be done by involving JsonReader and JsonWriter like it was in previous versions.

It would be nice to be able to choose between Newtonsoft.Json and System.Text.Json when using quicktype to create classes from Json.

Here is a good starting place to understand the changes I am refering to:
https://docs.microsoft.com/en-us/dotnet/core/whats-new/dotnet-core-3-0#fast-built-in-json-support

Cheers!

@dvdsgl
Copy link
Member

dvdsgl commented May 31, 2020

Love this idea! We would be happy to guide someone in contributing this as an option to the existing C# language support.

@Gambero81
Copy link

any news about supporting System.Text.Json?

@joelmartinez
Copy link

@dvdsgl ... I (potentially) volunteer as tribute!
image

Can you share any contribution guidelines/tips/insight? I'm in that brief window of time where I'm trying to decide if it would be easier to just change the generated code, or contribute this change to pay it forward :P

@dvdsgl
Copy link
Member

dvdsgl commented Mar 23, 2021

@joelmartinez do it! I'll help you if you get stuck. Should not be too bad.

See how we support many different serialization frameworks in Kotlin. Just make it an option, then we'll create some high-level helpers that emit the right code depending on which framework you're using.

@Jacko1394
Copy link

Can't wait for this!! 👍

@doggy8088
Copy link

@joelmartinez Do you have any progress on this issue?

@joelmartinez
Copy link

@doggy8088
image

Unfortunately, my "brief window of time" ended up going in another direction ... I never got quite deep enough into implementation territory here. So the task is still open for anyone else that wants a crack.

@CrAcK75
Copy link

CrAcK75 commented May 7, 2022

Any news?

@bzuidgeest
Copy link
Contributor

I got quicktype running with the code from the PR, seems to work fine as is for at least my needs. Is there any progress as to getting this included?

@doggy8088
Copy link

@bzuidgeest You might consider use my version of quicktype.

Source: https://github.com/doggy8088/quicktype/tree/CSharp-SystemTextJson

npm: @willh/quicktype

@bzuidgeest
Copy link
Contributor

@bzuidgeest You might consider use my version of quicktype.

Source: https://github.com/doggy8088/quicktype/tree/CSharp-SystemTextJson

npm: @willh/quicktype

I already made a version for myself based on your code. much nicer output then njsonschema.

I got it compiling and working on linux on an old laptop. But I can't seem to get it to compile on windows and that is where I need it. All kinds of weird errors with packages an dependencies. I develop in c#, nodejs and npm are alien to me. Though i could read and modify the typescript code. Its close to javascript en very readable.

I do wonder why you create a converter for enum?. .net 5 and 6 both have the build in stringenumconverter.

@doggy8088
Copy link

It because JsonStringEnumConverter doesn't fit my need on certain circumstance.

@bzuidgeest
Copy link
Contributor

It because JsonStringEnumConverter doesn't fit my need on certain circumstance.

That much was obvious. But not very helpfull. I do know it has an "issue" arround nullable enums, but personally I find enums should not be nullable, if you don't know the value a "unknown" option should be added to the enum. But opinions like that are personal and I cannot expect everyone to share them. Was this your "circumstance"?

@doggy8088
Copy link

doggy8088 commented Jul 19, 2022

@bzuidgeest I took some time to remember what's my circumstance. 😅

I have a json like this.

[
    {
        "name": "Will",
        "department": "CorpA.General",
        "age": "18"
    },
    {
        "name": "Mary",
        "department": "CorpB.Finance",
        "age": "19"
    }
]

The department must be an Enum type, but the value can't be a Enum options because there is a dot (.) in it. Let's why I can't use JsonStringEnumConverter as my default enum's converter.

Here is my sample code that can run under LINQPad 7.

void Main()
{
    var serializerOptions = new JsonSerializerOptions
    {
        Converters = {
            DepartmentConverter.Singleton
        },
        NumberHandling = JsonNumberHandling.AllowReadingFromString,
        WriteIndented = true
    };

    var jsonPath = Util.CurrentQueryPath.Replace(".linq", ".sample.json");
    var jsonText = File.ReadAllText(jsonPath);

    var user = JsonSerializer.Deserialize<UserProfile[]>(jsonText, serializerOptions);

    user.Dump();

    string json = JsonSerializer.Serialize(user, serializerOptions);

    json.Dump();
}

public class UserProfile
{
    [JsonPropertyName("name")]
    public string Name { get; set; }

    [JsonPropertyName("department")]
    public Department Department { get; set; }

    [JsonPropertyName("age")]
    public int Age { get; set; }
}

public enum Department
{
    CorpA_General,
    CorpB_Finance
};


public class DepartmentConverter : JsonConverter<Department>
{
    public override bool CanConvert(Type t) => t == typeof(Department);

    public override Department Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        var value = reader.GetString();
        switch (value)
        {
            case "CorpA.General":
                return Department.CorpA_General;
            case "CorpB.Finance":
                return Department.CorpB_Finance;
        }
        throw new Exception("Cannot unmarshal type Department");
    }

    public override void Write(Utf8JsonWriter writer, Department value, JsonSerializerOptions options)
    {
        switch (value)
        {
            case Department.CorpA_General:
                JsonSerializer.Serialize(writer, "CorpA.General");
                return;
            case Department.CorpB_Finance:
                JsonSerializer.Serialize(writer, "CorpB.Finance");
                return;
        }
        throw new Exception("Cannot marshal type Department");
    }

    public static readonly DepartmentConverter Singleton = new DepartmentConverter();
}

As this doc The Support enum string value deserialization:

It works without a specified naming policy or with the CamelCase naming policy. It doesn't support other naming policies.

My custom converter support every circumstances and make sure Read and Write are identical.

@dylanvdmerwe
Copy link

Would be amazing to have this option built in to replace Json.NET.

@bzuidgeest
Copy link
Contributor

@doggy8088 That seems like an excellent reason for doing it the way you did. The use case is rather specifc however (I think) and I don't know if it would hold up in a general use tool. What if the next person would use a different seperator and a different way of correcting it in the conversion. For example what if the replaced the dot in the enum by removing it al together? You would have to define a lot of use cases.

But thanks for explaining, that is very usefull.

@bzuidgeest
Copy link
Contributor

@dylanvdmerwe Replace is a bad option. A lot of people have invested in the code as-is and rely on it. Also JSON.net is far from dead. It might fade eventually, but for now its still heavily used and has unique features that are not in System.Text.Json and might never be available there. Personally I am a proponent of what @dvdsgl suggested. The Kotlin generator has an option to choose the framework to be used. We should have that for C# to.

I looked into the code files and we could copy the way kotlin does it and then cobine @doggy8088 repo his generator code with the existing generator code. Some name clashes would need to be fixed, but it should not be to difficult. Basically two generators and a choice between the two.

@dylanvdmerwe
Copy link

@bzuidgeest agreed my apologies I meant that having the option to select which generator to use would be incredible.

@bzuidgeest
Copy link
Contributor

I took the code from the repo of @doggy8088 and merged it with the current version of quicktype using the kotlin generator as an example of how to implement selecting between frameworks. Just like @dvdsgl sugested
It seems to work but I need to mention some caveats.

  • I am NO typescript developer. But code is code so I think I did a reasonable job putting everything in place.
  • The csharp testsuite still runs OK, so at least I did not break the existing stuff. But I don't know how to add a test project that can test the SystemTextJson framework to the project. Someone will have to point out what needs to happen. Could be the test docker container provided needs to be supplied with a newer .NET version, new test project etc...

code is here:
https://github.com/bzuidgeest/quicktype/tree/SystemTextJson
I could make a pull request, but I doubt that would be accepted without tests. Maybe I make one for show.

@bzuidgeest
Copy link
Contributor

So I committed number of changes and as far as I can determine most tests run fine. Even some that are excluded with newtonsoft lib because of framework issues. Seems reasonable to me. Lets hope someone actually want to take a look at the PR

@Bluscream
Copy link

Why is this not a thing yet? .net 6 does not even have newtonsoft

@bzuidgeest
Copy link
Contributor

bzuidgeest commented Oct 19, 2022

Why is this not a thing yet? .net 6 does not even have newtonsoft

That is nonsense, Newtonsoft Json.net library always has been a sperate lib and is very much supported on .NET6. Just as normal with the nuget package. The fact that Microsoft included it (the nuget) by default in some templates has no bearing on that.

With the latest .net you are free to choose between the system.text.json and newtonsoft libs.

That said, this project seems to be "on hold" as in the discussions you can read the maintainers do not have the time to maintain it. Wish I new that before I create my branch and PR.

dvdsgl added a commit that referenced this issue Jan 4, 2023
…1347 (#1997)

* Started modifying Charp.ts to have multiple framework support. Using kotlin as example.

* First version with new framework option is building

* Adjusted index.ts and removed some rundundant targetlanguage definitions

* Made some changes to get the charp language test to work. basically defaulting to newtonsoft as the test docker use a version of .net core that is to low.

* small fixes to render code to make tests compile

* Added .net6 sdk and fixed the test commands

* fix to remove warnings from console to be able to run tests.

* Most test work, 18 still to check and possibly fix.

* fixed code for test bug863.json. Missing options parameter on deserialisation

* fix all test with datetimeoffset parsing errors and 29f47

* in some test String.IsNullOrEmpty is not accepted but string.IsNullOrEmpty is.

* all  basic tests seem to run. Changed to general defaults for the serializer.

* Add csharp-SystemTextJson fixture

* Small C# fixes

* Bump versions

Co-authored-by: Bart Zuidgeest <bzuidgeest@outlook.com>
Co-authored-by: David Siegel <dvdsgl@users.noreply.github.com>
Co-authored-by: David Siegel <djsiegel@gmail.com>
@dvdsgl
Copy link
Member

dvdsgl commented Jan 4, 2023

Fixed in #1997

@dvdsgl dvdsgl closed this as completed Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants