Replaced NewtonSoft.Json with System.Text.Json

[hartmark]

Resolves #364.

[abatishchev]

Yay, it's working! At least it compiles :) But now tests fail, and looks like because of the differences between the two serializers.

Here's the list of failed tests: https://dev.azure.com/abatishchev/OpenSource/_build/results?buildId=1081&view=ms.vss-test-web.build-test-results-tab

[hartmark]

I added the missing serializer/deserializer for Dictionary<string, object> so it gets the same behaviour as Json.net.

It passes the tests* now here on my arch linux using rider.

*= 19 tests fail on Linux, but they did that before this change too. I'll investigate further when I get the time.

[abatishchev]

Can you please rebase the branch onto the latest main?

[hartmark]

I have rebased now, can you double-check if I have missed something. You can squash the commits after that so we have a cleaner change-log if we need to rebase from main again.

[abatishchev]

Build failed with a cryptic error:

##[error]C:\hostedtoolcache\windows\dotnet\sdk\6.0.300\Microsoft.Common.CurrentVersion.targets(1221,5): Error MSB3644: The reference assemblies for .NETFramework,Version=v3.5 were not found. To resolve this, install the Developer Pack (SDK/Targeting Pack) for this framework version or retarget your application. You can download .NET Framework Developer Packs at https://aka.ms/msbuild/developerpacks

Since it's a branch in your fork, it's bit tricky to clone it locally. But I should be able to do that, will give it a try.

[abatishchev]

I fixed the build an now only tests failing are in AspNetCore extensions project. Does it need any additional configuration?

[abatishchev]

Fixed the tests, had to make a breaking change in AspNetCore extensions.

Ideally we need to update tests to test both serialziers to make sure we don't break either but as it's currently done - the new serialzier is hard-coded for JWT that targets the modern .NET versions.

[hartmark]

Great find about the breaking test. I think this is a good change however as this allows to have this kind of claim.

{
 "fooClaim": { "property1" : "value", "property2" : "value2" ...
}

[hartmark]

I have added a method to IJsonSerializer to instruct if the payload should be serialized as camelCase or PascalCase, I hadded some tests for it too.

I however noticed that the old test projects didn't trigger on the NotImplementedException in JsonNetSerializer.

I saw that the test projects had all set net462 as their TargetFramework, changing it to a version lover than net462 triggered the exception for JsonNetSerializer.

What's the ambition of supporting old .net frameworks?
According to Microsoft everything below .net fw 4.6.2 is EOL.
https://docs.microsoft.com/en-us/lifecycle/products/microsoft-net-framework

I think that as long as it still works on old .net frameworks it doesn't hurt that this supports them.
But, on the flip side. If we just support .net fw 4.6.2 we can just drop Newtonsoft completely as System.Text.Json supports .net fw 4.6.2 as lowest.

[hartmark]

I have added another test for encoding using fluent call, It seems to fail encoding properly.

I need to go to bed now so I'll push and leave it up here if you find anything. There's end of school term tomorrow so I don't know how much time I have to investigate further this weekend.

Plus we should perhaps add the license for the SystemTextJsonDictionaryStringObjectJsonConverter class I added:
https://github.com/joseftw/JOS.SystemTextJsonDictionaryStringObjectJsonConverter/blob/develop/LICENSE

[abatishchev]

I have added a method to IJsonSerializer to instruct if the payload should be serialized as camelCase or PascalCase, I hadded some tests for it too.

Let's don't expose such property on the interface, it's too implementation specific, we should be able to set it somehow else. But main question - can you explain why it's needed?

What's the ambition of supporting old .net frameworks? According to Microsoft everything below .net fw 4.6.2 is EOL.

Yes, I know it's EOL already. But the support was added and kept intentionally. Every time I attempted to drop older frameworks - users would raise an issue, ask to put it back. This library has a relatively-small user base so I'd like to cater to everyone who still uses it. Keeping .NET 3.5/4.0 around didn't bother me so far. Meanwhile it's ok to exclude them from newer features if necessary.

Overall I'd suggest to maybe split the PR as it grows in the amount and complexity of the changes.

[abatishchev]

Here's the trick that sets the target framework to .NET 3.5/4.0:

 <ItemGroup>
    <ProjectReference Include="..\..\src\JWT\JWT.csproj">
      <SetTargetFramework>TargetFramework=net40</SetTargetFramework>
    </ProjectReference>
  </ItemGroup>

I verified yet another time recently, the resulting binaries do indeed reference the BCL of the said version.

[hartmark]

Let's don't expose such property on the interface, it's too implementation specific, we should be able to set it somehow else. But main question - can you explain why it's needed?

We have a third party component that expects the JSON in payload to be camel cased.

For JSON.net you just do this to det camel cased:
https://stackoverflow.com/a/22483122

Yes, I know it's EOL already. But the support was added and kept intentionally. Every time I attempted to drop older frameworks - users would raise an issue, ask to put it back. This library has a relatively-small user base so I'd like to cater to everyone who still uses it. Keeping .NET 3.5/4.0 around didn't bother me so far. Meanwhile it's ok to exclude them from newer features if necessary.

Fair enough, good points.

[hartmark]

Here's the trick that sets the target framework to .NET 3.5/4.0:

 <ItemGroup>
    <ProjectReference Include="..\..\src\JWT\JWT.csproj">
      <SetTargetFramework>TargetFramework=net40</SetTargetFramework>
    </ProjectReference>
  </ItemGroup>

I verified yet another time recently, the resulting binaries do indeed reference the BCL of the said version.

Alright, but it still choses the System.Text.Json serializer as you can see at the failed test on .net40 test-project

[abatishchev]

We have a third party component that expects the JSON in payload to be camel cased.

Something similar is covered in this doc: https://github.com/jwt-dotnet/jwt/blob/main/README.md#custom-json-serializer=

Few options/idea:

1. InheritSystemTextSerializer and redefine the settings. I've unsealed both serializers to make doing that easier.
2. Provide CamelCase version of both out-of-the-box
3. Add method to JsonSerializerFactory that would accept the corresponding settings object, make the factory public, add method to builder that accepts either factory or settings.

Please also update the doc.

Alright, but it still choses the System.Text.Json serializer as you can see at the failed test on .net40 test-project

Do these two projects need to define a constant so they would pick up the right serializer?

[hartmark]

I will try to come up with a solution for the serializer settings. We should perhaps use the ms dependency injection nuget as it seems the tests now fail that the settings have changed on a already used serializer.

If we use a DI component is easy to setup that it should use a new instance per usage

[hartmark]

I can move the serializer settings to another PR so it's less code changed at once

[hartmark]

Removed last commit

[hartmark]

I have merged with latest from main as this branch had conflicts. I don't know the versioning rules so I just used same version as in main.

Should we squash the commits and try to get this merged into main?

[abatishchev]

I don't know the versioning rules so I just used same version as in main

Please bump to the next beta version. That would be 10.0.0-beta3

Should we squash the commits

Up to you, I'll squash them on the PR completion anyway

and try to get this merged into main?

Sorry for making you waiting! I appreciate your contribution and the following up too. I didn't forget, just didn't have enough time yet (and empty enough mind; I have a very cool but also a very demanding job, which keeps me preoccupied more than I wish it would) to go through the changes for another, final review. Hopefully will have time by the end of this weekend.

Thanks again for the collaboration, I'll review and approve the change as soon as I can.

[hartmark]

No worries, I tend to keep myself occupied with other stuff too sometimes.

I'll squash the commits after work so we have more easily overview over the pr

[hartmark]

I have squashed and bumped the version now, please check if I have missed anything

[abatishchev]

Yay, awesome! Greatly appreciate your contribution, also your persistence and for bearing with me until we've made it happen.

Will push to NuGet shortly.

[hartmark]

It was a fun collaboration and I guess I have been under-stimulated with coding at work so this was a nice challenge :)