Remove the Newtonsoft.Json package and migrate to System.Text.Json

[EngRajabi]

In this request, we deleted the newtonsoft package and migrated to text json system.
The reason for the changes

• Newtonsoft package is no longer developed
• System text json package was developed by Microsoft itself and has many changes and improvements day by day
• System text json has a much better performance
• It consumes much less RAM
  The changes given include the removal of Newtonsoft. A benchmark has also been added for this

@MohammadAminPourmoradian helped me with this

BenchmarkDotNet v0.13.11, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3)
Intel Core i7-10870H CPU 2.20GHz, 1 CPU, 16 logical and 8 physical cores
.NET SDK 8.0.303
[Host] : .NET 6.0.32 (6.0.3224.31407), X64 RyuJIT AVX2 [AttachedDebugger]
.NET 8.0 : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2

Job=.NET 8.0 Runtime=.NET 8.0

Method	Count	Mean	Error	StdDev	Median	Op/s	Gen0	Gen1	Gen2	Allocated
MicrosoftDeserializeBigData	1000	856.3 us	53.98 us	157.47 us	797.1 us	1,167.8	39.0625	13.6719	-	328.78 KB
NewtonsoftDeserializeBigData	1000	1,137.2 us	18.74 us	17.53 us	1,132.8 us	879.4	54.6875	17.5781	-	457.94 KB
MicrosoftSerializeBigData	1000	646.4 us	12.72 us	20.90 us	645.7 us	1,546.9	110.3516	110.3516	110.3516	350.02 KB
NewtonsoftSerializeBigData	1000	1,033.4 us	19.37 us	42.53 us	1,022.8 us	967.7	109.3750	109.3750	109.3750	837.82 KB

[raman-m]

For the moment, I would appreciate it if you could eliminate all non-substantive changes such as:

• End of line characters
• Fixes to whitespace issues
• Formatting corrections

If this cannot be done, please inform me, and I will assist you. Let's concentrate on the actual changes.

[raman-m]

build - Success

Very well❕

[raman-m]

Thank you for the excellent benchmark testing with the new JsonSerializerBenchmark class!

I'm concerned that a direct upgrade to the Microsoft native library might work, but it could cause issues or even breaking changes for our development consumers. Therefore, we need to seek a more advanced and gentle solution through redesign.

My suggestions are:

• Maintain the old Newtonsoft.Json reference indefinitely for backward compatibility.
• Implement the new System.Text.Json functionality and set it as the default, as your PR suggests.
• Create a new Ocelot.Infrastructure.JsonUtil helper to establish a concrete dependency on the consumed library (requires a new Core infrastructure interface).
• Introduce a new global JSON setting in the GlobalConfiguration of ocelot.json, such as JsonOptions and a Type property, to choose between using Newtonsoft.Json or System.Text.Json libraries.
• Instantiate and delegate serialization operations to the actual parser instance.
• Inject the new interface object into all consuming classes through the DI mechanism.

This method is more adaptable: we maintain backward compatibility while gradually transitioning to Microsoft's implementations to enhance performance.

[raman-m]

I'm hesitant to rewrite the documentation until all the code has been verified and approved.

[raman-m]

I do not believe we have the authority to change the default settings of the library. Providing recommendations in the documentation to the community should have reasonable goals!
I do not think all Ocelot solutions on the Internet should adhere to these JSON options.

[EngRajabi]

This change does not change the library's behavior, so changing this method does not cause any problems.

[raman-m]

I understand that this sample might contain outdated code, yet it's essential to convert it into something usable. The sample illustrates how to override the default services in the DI feature. We can demonstrate how to revert to the old NewtonSoft services using new package versions. However, the rewriting should be deferred until all C# code is approved. Ultimately, updating the documentation should be the final step in the PR process.

[raman-m]

Well... The new JsonSerializer is from the System.Text.Json namespace.
But the package depends on Ocelot one and it consumes Ocelot interfaces.
Such hard changes constitute a major upgrade of the Ocelot Core and may introduce a breaking change, which will be detailed in the Release Notes.
I believe we need a more advanced solution.
Finally, the package should depend on Ocelot interfaces only!

[raman-m]

@EngRajabi @MohammadAminPourmoradian Mohsen and Mohammad, congratulations! The PR has entered the official code review stage. The build is green and stable now. I've made some on-the-fly code review adjustments, including superficial changes. The pull request is now well-prepared for an easy review.
Could you consider working on the suggestions from my code review plz?

@ggnaegi @RaynaldM Hello, I anticipate your thorough code review due to the significant and important upgrade in the Core involving the replacement of the JSON parsing library. I would value your consideration of my proposal to redesign the current draft solution, as I foresee potential issues.

[EngRajabi]

@raman-m requested changes on Jul 22, 2024

You gave a good suggestion, but is it really that much work?
Microsoft package system text json is executed on net 6 7 8. Old versions are not affected because json behavior has not changed.
It can be done by separating a serializer layer, but is it really necessary? How often does it change?

[ggnaegi]

@raman-m let's check this...

[raman-m]

JsonPath.Net reference must be removed❗

By including this library reference, you are breaching our Development Process for the Ocelot core package! Refer to point 5:

The main Ocelot package must not have taken on any non MS dependencies.

The significant issue with this PR is the included reference. It is essential to eliminate this reference and begin utilizing the full capabilities of native Microsoft functionality: native .NET classes or native target lib.

[raman-m]

@MohammadAminPourmoradian
After the v23.3 Hotfixes release the feature branch must be rebased onto develop and all merge-conflicts must be resolved.

[EngRajabi]

JsonPath.Net reference must be removed❗

By including this library reference, you are breaching our Development Process for the Ocelot core package! Refer to point 5:

The main Ocelot package must not have taken on any non MS dependencies.

The significant issue with this PR is the included reference. It is essential to eliminate this reference and begin utilizing the full capabilities of native Microsoft functionality: native .NET classes or native target lib.

I replaced

[raman-m]

Successor to #1183

• #651 Merge custom JSON properties across multiple ocelot.X.json configuration files using Newtonsoft's JToken merge functionality #1183

Congratulations, guys! 🎉 We are dependent on PR #1183, which is slated for merging first❗

[raman-m]

@EngRajabi @MohammadAminPourmoradian Sorry for merge conflicts! Please fix them by rebasing feature branch onto develop one!

[ggnaegi]

@EngRajabi @raman-m Who should resolve the merge conflicts?

[raman-m]

@ggnaegi Authors, indeed. However, it is well-known that most of our authors tend to be lazy with only a few exceptions.

Second, if an author does not respond and develop their PR, then as team members, we can deprioritize the PR and put it into waiting status.

Third, if the PR is accepted and included in a current release, then authors should be online. If they are offline, a team member takes responsibility to develop and deliver it, finally merging to the develop branch.

Are these good instructions?

Finally, the PR is not added to any milestones. I believe it should be developed further.
Additionally, the PR has the dependency ❗

[EngRajabi]

I resolved the conflicts.
@raman-m @ggnaegi

[coveralls]

coverage: 93.469% (-0.03%) from 93.503%
when pulling df7350d on EngRajabi:feat_rm_newtonsoft
into 9fc4e78 on ThreeMammals:develop.