From ff1b58585253525eae61214e7b5e44fea0cabac5 Mon Sep 17 00:00:00 2001 From: Yogesh Prajapati Date: Sat, 30 May 2026 21:13:44 +0100 Subject: [PATCH] Batch-5: fix #668 STJ JsonElement unwrap, #676 docs cleanup, document #692 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit for STJ, properties on an ExpandoObject built from JSON came through as JsonElement values. Rule expressions like `input1.country == "india"` then failed with "binary operator Equal is not defined for the types 'JsonElement' and 'System.String'." Utils.CreateAbstractClassType now infers the native CLR type from a JsonElement's ValueKind (string / int / long / double / bool / null), and Utils.CreateObject unwraps JsonElement scalars to their native values before assigning them. Objects and arrays inside the JsonElement keep JsonElement shape — that path is for typed Newtonsoft-style models that weren't using ExpandoObject anyway. in JSON examples across README.md, docs/Getting-Started.md, and docs/index.md. Removed. after 5.0.4." That's actually standard C# Nullable semantics — both `null < x` and `null > x` are false. The pre-5.0.4 behavior was a Newtonsoft / Dynamic.Core quirk, not the documented contract. Added an explicit test documenting the current behavior plus the canonical `!Dt.HasValue || Dt < someDate` workaround for users who want null-aware ordering. Not changed here: #679 (rule-chaining via @RuleName) is already covered by in-flight PR #680 — no point duplicating that work. #710 and #709 are unrelated AGV-themed spam; closing comments are in issue-close-comments-batch-5.md. All 164 unit tests pass on net6 / net8 / net9 / net10. --- CHANGELOG.md | 9 +++ README.md | 2 - docs/Getting-Started.md | 5 -- docs/index.md | 3 - src/RulesEngine/HelperFunctions/Utils.cs | 57 ++++++++++++-- test/RulesEngine.UnitTest/Issue668Test.cs | 56 ++++++++++++++ test/RulesEngine.UnitTest/Issue692Test.cs | 90 +++++++++++++++++++++++ 7 files changed, 207 insertions(+), 15 deletions(-) create mode 100644 test/RulesEngine.UnitTest/Issue668Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue692Test.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index beaf7f00..cc6524eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ All notable changes to this project will be documented in this file. - New `ReSettings.AutoExecuteActions` (default `true`). Set to `false` to evaluate rules without automatically running their OnSuccess/OnFailure actions, so callers can run actions selectively via `ExecuteActionWorkflowAsync` (#596). - Documented and tested passing computed `additionalInputs` into the `EvaluateRule` action — the additionalInput `Name` is referenced directly in the target rule's expression (#573). +### Fixes +- `Utils.CreateAbstractClassType` / `CreateObject` now unwrap `System.Text.Json.JsonElement` scalar values to their native CLR types (string / int / long / double / bool / null) when building typed objects from `ExpandoObject` inputs. This restores the pre-System.Text.Json behavior for rule expressions like `input1.country == "india"` that previously failed with "binary operator Equal is not defined for the types 'JsonElement' and 'String'" (#668). + +### Docs +- Removed the obsolete `ErrorType` field from JSON examples in `README.md`, `docs/Getting-Started.md`, and `docs/index.md`. `ErrorType` was removed from the `Rule` model in 4.0.0 (#676). + +### Regression guards added (already correct on master, now covered by tests) +- #692 — Nullable `DateTime` comparisons against `null` (`null < someDate` / `null > someDate`) return `false`, matching standard C# `Nullable` semantics. Test documents the recommended `HasValue` workaround for users who want null-aware ordering. + ## [6.0.1-preview.1] ### Performance diff --git a/README.md b/README.md index 291cba55..4fdd9c18 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,6 @@ An example rule: "RuleName": "GiveDiscount10", "SuccessEvent": "10", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.country == \"india\" AND input1.loyaltyFactor <= 2 AND input1.totalPurchasesToDate >= 5000" }, @@ -39,7 +38,6 @@ An example rule: "RuleName": "GiveDiscount20", "SuccessEvent": "20", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.country == \"india\" AND input1.loyaltyFactor >= 3 AND input1.totalPurchasesToDate >= 10000" } diff --git a/docs/Getting-Started.md b/docs/Getting-Started.md index 756e1157..23122163 100644 --- a/docs/Getting-Started.md +++ b/docs/Getting-Started.md @@ -19,7 +19,6 @@ Rules schema is available in the [schema file](https://github.com/microsoft/Rule "RuleName": "GiveDiscount10", "SuccessEvent": "10", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.country == \"india\" AND input1.loyalityFactor <= 2 AND input1.totalPurchasesToDate >= 5000 AND input2.totalOrders > 2 AND input3.noOfVisitsPerMonth > 2" }, @@ -27,7 +26,6 @@ Rules schema is available in the [schema file](https://github.com/microsoft/Rule "RuleName": "GiveDiscount20", "SuccessEvent": "20", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.country == \"india\" AND input1.loyalityFactor == 3 AND input1.totalPurchasesToDate >= 10000 AND input2.totalOrders > 2 AND input3.noOfVisitsPerMonth > 2" }, @@ -35,7 +33,6 @@ Rules schema is available in the [schema file](https://github.com/microsoft/Rule "RuleName": "GiveDiscount25", "SuccessEvent": "25", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.country != \"india\" AND input1.loyalityFactor >= 2 AND input1.totalPurchasesToDate >= 10000 AND input2.totalOrders > 2 AND input3.noOfVisitsPerMonth > 5" }, @@ -43,7 +40,6 @@ Rules schema is available in the [schema file](https://github.com/microsoft/Rule "RuleName": "GiveDiscount30", "SuccessEvent": "30", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.loyalityFactor > 3 AND input1.totalPurchasesToDate >= 50000 AND input1.totalPurchasesToDate <= 100000 AND input2.totalOrders > 5 AND input3.noOfVisitsPerMonth > 15" }, @@ -51,7 +47,6 @@ Rules schema is available in the [schema file](https://github.com/microsoft/Rule "RuleName": "GiveDiscount35", "SuccessEvent": "35", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.loyalityFactor > 3 AND input1.totalPurchasesToDate >= 100000 AND input2.totalOrders > 15 AND input3.noOfVisitsPerMonth > 25" } diff --git a/docs/index.md b/docs/index.md index d3d0a550..27ced41b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -356,7 +356,6 @@ Define OnSuccess or OnFailure Action for your Rule: "RuleName": "GiveDiscount10Percent", "SuccessEvent": "10", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.couy == \"india\" AND input1.loyalityFactor <= 2 AND input1.totalPurchasesToDate >= 5000 AND input2.totalOrders > 2 AND input2.noOfVisitsPerMonth > 2", "Actions": { @@ -414,7 +413,6 @@ Define OnSuccess or OnFailure Action for your Rule: "RuleName": "GiveDiscount10Percent", "SuccessEvent": "10", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.couy == \"india\" AND input1.loyalityFactor <= 2 AND input1.totalPurchasesToDate >= 5000 AND input2.totalOrders > 2 AND input2.noOfVisitsPerMonth > 2", "Actions": { @@ -515,7 +513,6 @@ Actions can have async code as well "RuleName": "GiveDiscount10Percent", "SuccessEvent": "10", "ErrorMessage": "One or more adjust rules failed.", - "ErrorType": "Error", "RuleExpressionType": "LambdaExpression", "Expression": "input1.couy == \"india\" AND input1.loyalityFactor <= 2 AND input1.totalPurchasesToDate >= 5000 AND input2.totalOrders > 2 AND input2.noOfVisitsPerMonth > 2", "Actions": { diff --git a/src/RulesEngine/HelperFunctions/Utils.cs b/src/RulesEngine/HelperFunctions/Utils.cs index b13af66d..9bd0f53a 100644 --- a/src/RulesEngine/HelperFunctions/Utils.cs +++ b/src/RulesEngine/HelperFunctions/Utils.cs @@ -35,10 +35,11 @@ public static Type CreateAbstractClassType(dynamic input) if (input is System.Text.Json.JsonElement jsonElement) { - if (jsonElement.ValueKind == System.Text.Json.JsonValueKind.Null) - { - return typeof(object); - } + // STJ leaves scalar properties as JsonElement values. Infer the native CLR type + // so member comparisons in rule expressions (e.g. `input1.country == "india"`) + // work without users seeing "binary operator Equal is not defined for the types + // 'JsonElement' and 'String'." See #668. + return InferJsonElementClrType(jsonElement); } else if (input == null) { @@ -68,6 +69,43 @@ public static Type CreateAbstractClassType(dynamic input) return type; } + // Maps a JsonElement to the CLR type its scalar value would have once unwrapped. + // Objects and arrays keep their JsonElement type — callers using objects/arrays in + // expressions are already on a typed-path with Newtonsoft-style models. See #668. + private static Type InferJsonElementClrType(System.Text.Json.JsonElement el) + { + switch (el.ValueKind) + { + case System.Text.Json.JsonValueKind.String: return typeof(string); + case System.Text.Json.JsonValueKind.True: + case System.Text.Json.JsonValueKind.False: return typeof(bool); + case System.Text.Json.JsonValueKind.Number: + if (el.TryGetInt32(out _)) return typeof(int); + if (el.TryGetInt64(out _)) return typeof(long); + return typeof(double); + case System.Text.Json.JsonValueKind.Null: + case System.Text.Json.JsonValueKind.Undefined: return typeof(object); + default: return typeof(System.Text.Json.JsonElement); + } + } + + private static object UnwrapJsonElementScalar(System.Text.Json.JsonElement el) + { + switch (el.ValueKind) + { + case System.Text.Json.JsonValueKind.String: return el.GetString(); + case System.Text.Json.JsonValueKind.True: return true; + case System.Text.Json.JsonValueKind.False: return false; + case System.Text.Json.JsonValueKind.Number: + if (el.TryGetInt32(out var i)) return i; + if (el.TryGetInt64(out var l)) return l; + return el.GetDouble(); + case System.Text.Json.JsonValueKind.Null: + case System.Text.Json.JsonValueKind.Undefined: return null; + default: return el; + } + } + // Returns the CLR List type that should represent a heterogeneous IList of ExpandoObject / // IDictionary elements. Walks every element so properties that only appear in // later elements are still included in the generated type. See #704. @@ -158,11 +196,20 @@ public static object CreateObject(Type type, dynamic input) }; val = newList; } + else if (expando.Value is System.Text.Json.JsonElement je) + { + // Unwrap scalar JsonElement to its native value so the typed property + // receives a string/int/bool/etc. rather than a JsonElement. See #668. + val = UnwrapJsonElementScalar(je); + } else { val = expando.Value; } - propInfo.SetValue(obj, val, null); + if (val != null) + { + propInfo.SetValue(obj, val, null); + } } } diff --git a/test/RulesEngine.UnitTest/Issue668Test.cs b/test/RulesEngine.UnitTest/Issue668Test.cs new file mode 100644 index 00000000..10367fd3 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue668Test.cs @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Dynamic; +using System.Text.Json; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue668Test + { + // Reporter scenario: JSON inputs deserialized via System.Text.Json end up as + // JsonElement values inside an ExpandoObject. Rule expressions then compare + // those JsonElements against strings/numbers and fail with: + // The binary operator Equal is not defined for the types + // 'System.Text.Json.JsonElement' and 'System.String'. + // + // The migration from Newtonsoft.Json to System.Text.Json (#599) is the cause — + // Newtonsoft produced native .NET types into the ExpandoObject; STJ produces + // JsonElement. + [Fact] + public async Task ExpandoObject_WithJsonElementProperty_ComparedToString() + { + // Mirrors what STJ does when deserializing a JSON object into an ExpandoObject + // via a JsonDocument: each property becomes a JsonElement. + var json = "{\"country\":\"india\",\"loyaltyFactor\":2}"; + using var doc = JsonDocument.Parse(json); + + // Build an ExpandoObject with JsonElement values, the way #599 would. + IDictionary expando = new ExpandoObject(); + foreach (var prop in doc.RootElement.EnumerateObject()) + { + expando[prop.Name] = prop.Value.Clone(); // JsonElement, NOT a string + } + + var workflow = new Workflow + { + WorkflowName = "Discount", + Rules = new[] { + new Rule { RuleName = "R", Expression = "input1.country == \"india\"" } + } + }; + var engine = new RulesEngine(new[] { workflow }); + var results = await engine.ExecuteAllRulesAsync( + "Discount", new[] { RuleParameter.Create("input1", (ExpandoObject)expando) }); + + Assert.True(results[0].IsSuccess, + $"Expected success. Got ExceptionMessage = {results[0].ExceptionMessage}"); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue692Test.cs b/test/RulesEngine.UnitTest/Issue692Test.cs new file mode 100644 index 00000000..f46fafa4 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue692Test.cs @@ -0,0 +1,90 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue692Test + { + public class WithNullableDate + { + public DateTime? Dt { get; set; } + } + + // The reporter says: when comparing a null DateTime against a set DateTime, BOTH + // `<` and `>` return false (whereas in 5.0.3 and earlier null was treated as + // "less than" any set datetime). + // + // Behavior of standard .NET nullable DateTime semantics: + // null < someDateTime → false (Nullable comparisons return false when either operand is null) + // null > someDateTime → false + // This is the SAME as standard C# semantics — Nullable comparisons are tri-valued + // and false-when-null is the documented behavior. There is no "null is less than" rule + // in .NET; the reporter's previous behavior was either via Newtonsoft.Json string-typing + // (null treated as empty / default DateTime) or via a Dynamic.Core quirk. + // + // These tests document the current behavior so we don't accidentally regress. + [Fact] + public async Task NullableDateTime_LessThan_NullReturnsFalse() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Dt < DateTime.Now" } } + }; + var engine = new RulesEngine(new[] { workflow }, + new ReSettings { CustomTypes = new[] { typeof(DateTime) } }); + var results = await engine.ExecuteAllRulesAsync( + "wf", new[] { RuleParameter.Create("input1", new WithNullableDate { Dt = null }) }); + + Assert.False(results[0].IsSuccess); + } + + [Fact] + public async Task NullableDateTime_GreaterThan_NullReturnsFalse() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Dt > DateTime.Now" } } + }; + var engine = new RulesEngine(new[] { workflow }, + new ReSettings { CustomTypes = new[] { typeof(DateTime) } }); + var results = await engine.ExecuteAllRulesAsync( + "wf", new[] { RuleParameter.Create("input1", new WithNullableDate { Dt = null }) }); + + Assert.False(results[0].IsSuccess); + } + + // The canonical workaround for users who DO want null-aware semantics: check HasValue + // explicitly. This is also the standard C# pattern. + [Fact] + public async Task NullableDateTime_ExplicitHasValueCheck_WorksAsExpected() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { RuleName = "TreatNullAsLess", + Expression = "!input1.Dt.HasValue || input1.Dt < DateTime.Now" } + } + }; + var engine = new RulesEngine(new[] { workflow }, + new ReSettings { CustomTypes = new[] { typeof(DateTime) } }); + + var withNull = await engine.ExecuteAllRulesAsync( + "wf", new[] { RuleParameter.Create("input1", new WithNullableDate { Dt = null }) }); + Assert.True(withNull[0].IsSuccess); + + var withValue = await engine.ExecuteAllRulesAsync( + "wf", new[] { RuleParameter.Create("input1", new WithNullableDate { Dt = DateTime.Now.AddDays(-1) }) }); + Assert.True(withValue[0].IsSuccess); + } + } +}