TelegramBots / Telegram.Bot

.NET Client for Telegram Bot API
https://telegrambots.github.io/book
MIT License
3.09k stars 679 forks source link

Support System.Text.Json (as well as Newtonsoft.Json) #839

Open poulad opened 4 years ago

poulad commented 4 years ago

We can use new and high performance JSON serialization System.Text.Json package. Faster serialization/deserialization of request and response data structures is the main reason for this suggestion.

There are a few things to consider:

System.Text.Json requires .NET Standard 2.0 and that's .NET 4.6.1+.

Snake case naming policy support won't come until .NET 5 (see dotnet/corefx#41354). Alternatives are using camel cased naming policy by default, e.g. for username, and overriding specific fields, e.g. for first_name, with the [JsonPropertyName("first_name")].

Custom type converters such as for DateTime to Unix epoch time need to be implemented. This is very similar to the current implementation for Newtonsoft.Json.

References

The future of JSON in .NET Core 3.0

poulad commented 4 years ago

Ignoring Default Values

As @tuscen pointed out on #838, default value of the value types should be ignored during serialization but it's not supported at the moment. Default null value of the reference types can be ignored using the IgnoreNullValues in serialization options.

class Foo
{
    [JsonPropertyName("field_bar")]
    public bool FieldBar { get; set; }

    [JsonPropertyName("field_baz")]
    public string FieldBaz { get; set; }
}
[Fact]
public void Test()
{
    var opts = new JsonSerializerOptions { IgnoreNullValues = true };
    string json = JsonSerializer.Serialize(new Foo(), opts);
    // JSON is {"field_bar":false}

    // assertion fails
    Assert.Equal("{}", json);
}

see more at dotnet/corefx#40922

poulad commented 4 years ago

@tuscen :

...webhook users will need to set custom JsonSerializerOptions specifically for Bot API, because these options are global for the entire middleware pipeline in ASP.NET Core. If I'd want to host telegram webhook in the same process as my web app options needed for the library might not be compatible with the app. With the current setup we don't even need custom serializer options.

https://github.com/TelegramBots/Telegram.Bot/pull/838#discussion_r332608326

dumkin commented 2 years ago

Ignoring Default Values

As @tuscen pointed out on #838, default value of the value types should be ignored during serialization but it's not supported at the moment. Default null value of the reference types can be ignored using the IgnoreNullValues in serialization options.

class Foo
{
    [JsonPropertyName("field_bar")]
    public bool FieldBar { get; set; }

    [JsonPropertyName("field_baz")]
    public string FieldBaz { get; set; }
}
[Fact]
public void Test()
{
    var opts = new JsonSerializerOptions { IgnoreNullValues = true };
    string json = JsonSerializer.Serialize(new Foo(), opts);
    // JSON is {"field_bar":false}

    // assertion fails
    Assert.Equal("{}", json);
}

see more at dotnet/corefx#40922

dotnet 5&6 can do that now image

tuscen commented 2 years ago

@dumkin unfortunately this is not a solution because if someone would use the library with a webhook they could need different json options compared to what we need. And as far as I know json options are global in ASP.NET Core.

dumkin commented 2 years ago

I don't really understand at all why it is necessary to create a controller with an action with parameters. Why not just ask to add an additional route. And get everything you need from the body. And here it doesn't matter. use system.text.json or newtonsoft.json. it will not affect other libraries and user code

example for webhook image

dumkin commented 2 years ago

It's just that now, in essence, the situation is the same. Only the other way around. now it is required

services.AddControllers()
                    .AddNewtonsoftJson();

Which forces us to use NewtonsoftJson exactly as globally

tuscen commented 2 years ago

You have a point 🤔 We'll think about it.

peter-mghendi commented 2 years ago

This is something I would very much like in this library. Is there anything I can help with that would get this done quicker?

tuscen commented 2 years ago

@sixpeteunder Unfortunately , there's not much you can do. We rely heavily on:

All these features are still missing even in .NET 6.

Basically STJ is still mostly in MVP stage and is usable for APIs you control completely

Lonli-Lokli commented 11 months ago

I think things are changed in .net 8

klinki commented 8 months ago

This would be really great to have, it is a blocker for AOT.

Fedorus commented 8 months ago

This would be really great to have, it is a blocker for AOT.

Well, usage in Unity or front end blazor is strongly not recommended, so its not big of a deal )

klinki commented 8 months ago

True, but AOT is not just about raw performance but also about memory footprint. Especially trimming is really great to reduce memory footprint. I'm experimenting with TelegramBot client used for forex trading running on very low spec VPS provided by my forex broker. It has just 1GB RAM available and it has to run MT4 platform + my telegram bot. Currently it works, but sometimes I'm getting near the memory limit.

luizfbicalho commented 4 months ago

Whats the status on this? looks like there is a lot of problems to migrate

tuscen commented 4 months ago

@luizfbicalho All the changes are in the branch tuscen/stj-2 and I'd say it's 80-90% ready. I just don't have enough free time to squash all the bugs and finish it. I want to release Bot API 7.0 and 7.1 updates first and then release STJ support.

The thing is that it ended up quite non-trivial in a lot of places and it probably won't be backwards compatible so it'll be released as a new major version. The current plan is to completely remove Newtonsoft from net7.0 and newer targets and use built-in STJ from the runtime. All the targets below net7.0 will stay on Newtonsoft because earlier versions of STJ don't have some features that we use from Newtonsoft.

luizfbicalho commented 4 months ago

I have the same issues with my projects, thanks for the answer and I'll see it if I can help in any way

tuscen commented 4 months ago

Also the first release with STJ will not be AOT compatible. I want to implement it in a separate release, because otherwise it will take too much time.

tuscen commented 4 months ago

@luizfbicalho you can run unit tests from that branch and see which ones are failing. We also will need to add a few hundreds of serialization tests more to ensure that everything works as expected :)