asulwer / RulesEngine

Rules Engine with extensive Dynamic expression support
MIT License
19 stars 1 forks source link

Switch from Newtonsoft.Json to System.Text.Json #32

Closed RenanCarlosPereira closed 3 months ago

RenanCarlosPereira commented 3 months ago

Why the Change?

  1. Performance: System.Text.Json is faster and uses less memory.
  2. Built-in: It's part of .NET Core and .NET 5+, so fewer external dependencies.
  3. Security: Better default security settings.
  4. Future-proof: Regular updates from the .NET team.

What’s Different?

Other Details

Things to Watch Out For

RenanCarlosPereira commented 3 months ago

@abbasc52, could please also take a look at this PR if not asking too much?

I don't think changing from Newtonsoft to System.Text.Json will break anything, any thoughts?

asulwer commented 3 months ago

i referenced the reason why this shouldn't occur with this issue

Table of differences

abbasc52 commented 3 months ago

Json parsing is only relevent in 2 places, when user passes workflow as string and secondly in Action context parsing.(I do remember making Action context System.Text.Json compatible)

One major difference is System.Text.Json does not guess type while parsing without a type(by design) .This can lead to some corner cases which might need validation. I will go through the code and report if I see something obvious but old repo had a few asks to migrate and I probably provided reasons.

One way to go forward would be to uncouple json parser as separate extension packages.

Or newtonsoft and other changes can be part of a compatibility nuget, so that main project remains clean and people can migrate as per their convenience.

RenanCarlosPereira commented 3 months ago

Nice input @abbasc52

I can cover in unit tests the type s converts

Any ideas on how I can test it?

I don't see any issues moving to System.Text.Json in trying to think in a situation where I can cover in ActionContext

https://github.com/asulwer/RulesEngine/blob/8ba9be2cc303de39b323fa7431b686a04ab09aa0/src/RulesEngine/Actions/ActionContext.cs#L37