Dropping Dependence on Newtonsoft.Json

[bobbyangers]

Using this
How to migrate from Newtonsoft.Json to System.Text.Json

If .NET core 3.1 should be the minimum supported framework version supported; It would be a great time to drop Newtonsoft.Json and use System.Text.Json

In the code, there is only a few things to consider;

• Only used HttpEngine<T, TR> and in the two tests projects
• Remove the [JsonProperty] everywhere camelcase is by convention
• Remove the [JsonProperty] and replace with [System.Text.Json.JsonPropertyName] where properties are not conventional, like when using dashes.
• StringBooleanConverter Migrate to System.Text.Json
• DateRestrictJsonConverter Migrate to System.Text.Json
• SortExpressionJsonConverter Migrate to System.Text.Json
• StringEnumOrDefaultConverter Migrate to System.Text.Json
• StringEnumListConverterTest remove, convert to DayOfWeek ?

[vivet]

Hi
Let me think a bit about this. I kind of like Newtonsoft.

[bobbyangers]

That is your call or course;

Some points to consider;

• Serialization is faster with System.Text.Json
• Although Newtonsoft.Json was the standard, it's loosing traction in favor System.Text.Json
• Easier upgrade path ( one less dependency on third-party);
• Most other library are gradually dropping their support; Microsoft is strongly encouraged to drop Newtonsoft.Json

We are only use it in the HttpEngine; so relatively easy to remove.

[SykesJohn]

I like the idea of being able to drop the Newtonsoft.Json library. It is indeed a great product, and System.Text.Json may be a bit more complicated in spots, but System.Text.Json will always be there and supported.

[vivet]

I did a quick tryout, and it seems System.Text.Json is missing a way to specify converter type for serialization of tems in collections. This isn't planned before .NET 8, so I will wait for the full support.

dotnet/runtime#54189 (comment)

[rangers-globecar]

That is odd… but this project only uses converters via attributes anyway…. just saying.

[vivet]

Not sure what you mean.
It's the attribute to specify itemconvertertype that isn't available. Probably it's possible to implement it, but i prefer waiting for the support in .NET

[rangers-globecar]

Hi. I had successfully reimplemented the two custom converter and decoracted the
properties where appropriate, exactly as Newtonsoft equivalent. all good, all tests where passing.

But hey, it’s your decision.

BTW there was a security related bug with NewtonSoft just last month.

[vivet]

I will take a look at the PR

[vivet]

I have merged your PR.
I still need to do some refactoring and testing before I can release it.

[bobbyangers]

@vivet thank you for letting me know.

[vivet]

Will just keep the issue open until released

[vivet]

I did some testing, and mostly nothing works, so I have decided to rollback.
I will abort the project for now

[bobbyangers]

@vivet I did some testing and did encounter some issues:

• some enum values were missing and not parsing correctly
  • PlaceLocationTye >> Primary_School,
  • modified the CustomJsonStringEnumConverter to return the invalid value: ThrowJsonException($"Could not parse {enumString}");
  • missing language /Common/Language

    [EnumMember(Value = "lus")]
    Mizo

• some boolean values are now tried first before trying to parse a string
• fields that were supposed to be int were strings (public int Offset { get; set; })
• JsonSerializerOptions was adjusted
  • DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault
  • NumberHandling = JsonNumberHandling.AllowReadingFromString
  • SortExpressionJsonConverter now included, was missing

The FileType is defined as IEnumerable[Enum], however using the custom search, the value returned is a csv, fixed

Could not very the PlusCodeGeocode*

GoogleApi.Exceptions.GoogleApiException : RequestDenied: This IP, site or mobile application is not authorized to use this API key. Request received from IP address 66.102.8.95, with empty referer | https://plus.codes/api?key=[redacted]&address=285 Bedford Ave%2C Brooklyn%2C NY 11211%2C USA&language=en

And the SpeedLimits

GoogleApi.Exceptions.GoogleApiException : Response status code does not indicate success: 403 (Forbidden).|https://roads.googleapis.com/v1/speedLimits?key=[redacted]&path=60.17088%2C24.942795|60.170879%2C24.942796|60.170877%2C24.942796

And finally the PlacesTextSearchWhenPageTokenTest does not yield multiple pages from my location.

But now all other tests are now passing ....

[vivet]

Hi
I will try to test your branch soon :-)

[bobbyangers]

Sounds good.

However, I think I would need to rebase and adjust according to the new language enum converter...

I'll take a look at it later today.

[angelru]

Any advance?

[vivet]

Not really. I have been busy with other projects.
Though, it's good that you ask, cause i should really get this done 😄.
I will see to it during next week

[vivet]

I have merged the PR, and working on cosmetics refactoring and ensuring everything works. I expect release within 1-2 days

[marchy]

@vivet @bobbyangers Great job guys!

I think this is great that it is being taken on. All our 3rd party libraries have all been slowly moving over to it (ie: Airtable) – despite us not having taken it on in our own codebase yet.

Will provide some inspiration for us to move to it when the opportunity comes up =)

[vivet]

Ladies and Gentlemen, it's published
Version 4.3.0

I will close this issue.
Should you experience any issues, please create a new issue