From 5d7398367c255cbaa1ca9c537e6e496f09593905 Mon Sep 17 00:00:00 2001 From: Yogesh Prajapati Date: Wed, 27 May 2026 16:49:44 +0100 Subject: [PATCH] Fix #576, #519, #696, and add regression guards for #590, #608, #581, #606 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code fixes: (common in custom actions that don't need config). Added null guards for both the context dict itself and each entry's value. ErrorMessage template was never interpolated into ExceptionMessage on the result tree (unlike ExecuteAllRulesAsync which formats correctly). Added the call. `$(input1.Inner.Name)` produced the raw JSON of `Inner` (`{"Name":"..."}`) instead of walking to the leaf. Rewrote UpdateErrorMessage to traverse the full path via JsonNode and emit the leaf scalar (unquoted) or nested JSON. Verified-already-fixed (regression tests added): run was fixed in commit 4ca3cc2 (#592). Test guards against regression. no longer reproduces — likely fixed by FastExpressionCompiler / Dynamic.Core version bumps. Test guards against regression. no longer reproduces, likely resolved by the AutoRegisterInputType work. Test guards against regression. mis-resolved on master — likely fixed by Dynamic.Core 1.4.3 → 1.6.7 bump. Test guards against regression. Not fixed in this PR: decision rather than a bug — ActionContext is documented as a static key/value config dict. Users who want evaluation should use OutputExpression. All 142 unit tests pass on net6 / net8 / net9 / net10. --- src/RulesEngine/Actions/ActionContext.cs | 35 ++++++---- src/RulesEngine/RulesEngine.cs | 51 ++++++++------ test/RulesEngine.UnitTest/Issue519Test.cs | 62 +++++++++++++++++ test/RulesEngine.UnitTest/Issue576Test.cs | 69 +++++++++++++++++++ test/RulesEngine.UnitTest/Issue581Test.cs | 50 ++++++++++++++ test/RulesEngine.UnitTest/Issue590Test.cs | 65 ++++++++++++++++++ test/RulesEngine.UnitTest/Issue606Test.cs | 47 +++++++++++++ test/RulesEngine.UnitTest/Issue608Test.cs | 81 +++++++++++++++++++++++ test/RulesEngine.UnitTest/Issue696Test.cs | 75 +++++++++++++++++++++ 9 files changed, 502 insertions(+), 33 deletions(-) create mode 100644 test/RulesEngine.UnitTest/Issue519Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue576Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue581Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue590Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue606Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue608Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue696Test.cs diff --git a/src/RulesEngine/Actions/ActionContext.cs b/src/RulesEngine/Actions/ActionContext.cs index 44116679..9db6203e 100644 --- a/src/RulesEngine/Actions/ActionContext.cs +++ b/src/RulesEngine/Actions/ActionContext.cs @@ -17,22 +17,31 @@ public class ActionContext public ActionContext(IDictionary context, RuleResultTree parentResult) { _context = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var kv in context) + if (context != null) { - string key = kv.Key; - string value; - switch (kv.Value.GetType().Name) + foreach (var kv in context) { - case "String": - case "JsonElement": - value = kv.Value.ToString(); - break; - default: - value = JsonSerializer.Serialize(kv.Value); - break; - + string key = kv.Key; + string value; + if (kv.Value == null) + { + value = null; + } + else + { + switch (kv.Value.GetType().Name) + { + case "String": + case "JsonElement": + value = kv.Value.ToString(); + break; + default: + value = JsonSerializer.Serialize(kv.Value); + break; + } + } + _context.Add(key, value); } - _context.Add(key, value); } _parentResult = parentResult; } diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index 3c125673..b1627063 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -132,6 +132,9 @@ public async ValueTask ExecuteActionWorkflowAsync(string workf var compiledRule = CompileRule(workflowName, ruleName, ruleParameters); var extendedRuleParameters = EvaluateGlobalsAdHoc(workflowName, ruleParameters); var resultTree = compiledRule(extendedRuleParameters); + // Mirror ExecuteAllRulesAsync's behavior: format the per-rule ErrorMessage template + // into ExceptionMessage before any action runs / before returning. See #519. + FormatErrorMessages(new[] { resultTree }); var actionResult = await ExecuteActionForRuleResult(resultTree, true); ThrowIfActionExceptionShouldPropagate(actionResult, resultTree); return actionResult; @@ -541,9 +544,7 @@ private IEnumerable FormatErrorMessages(IEnumerable 1) { - var typeName = property?.Split('.')?[0]; - var propertyName = property?.Split('.')?[1]; - errorMessage = UpdateErrorMessage(errorMessage, inputs, property, typeName, propertyName); + errorMessage = UpdateErrorMessage(errorMessage, inputs, property); } else { @@ -562,28 +563,38 @@ private IEnumerable FormatErrorMessages(IEnumerable - /// Updates the error message. + /// Resolves a dotted-path placeholder like $(input1.Inner.Name) against the rule inputs, + /// walking arbitrary depth. See #696. /// - /// The error message. - /// The evaluated parameters. - /// The property. - /// Name of the type. - /// Name of the property. - /// Updated error message. - private static string UpdateErrorMessage(string errorMessage, IDictionary inputs, string property, string typeName, string propertyName) + private static string UpdateErrorMessage(string errorMessage, IDictionary inputs, string property) { - var arrParams = inputs?.Select(c => new { Name = c.Key, c.Value }); - var model = arrParams?.Where(a => string.Equals(a.Name, typeName))?.FirstOrDefault(); - if (model != null) + var segments = property.Split('.'); + var typeName = segments[0]; + var model = inputs?.FirstOrDefault(c => string.Equals(c.Key, typeName)); + if (model?.Value == null) + { + return errorMessage; + } + + var modelJson = JsonSerializer.Serialize(model.Value.Value); + JsonNode current = JsonNode.Parse(modelJson); + for (var i = 1; i < segments.Length && current != null; i++) + { + current = current is JsonObject jObj && jObj.TryGetPropertyValue(segments[i], out var next) + ? next + : null; + } + + if (current == null) { - var modelJson = JsonSerializer.Serialize(model?.Value); - var jObj = JsonObject.Parse(modelJson).AsObject(); - JsonNode jToken = null; - var val = jObj?.TryGetPropertyValue(propertyName, out jToken); - errorMessage = errorMessage.Replace($"$({property})", jToken != null ? jToken?.ToString() : $"({property})"); + return errorMessage; } - return errorMessage; + // JsonValue (leaf scalar) should render without quotes; objects/arrays render as JSON. + var replacement = current is JsonValue v && v.TryGetValue(out var stringValue) + ? stringValue + : current.ToString(); + return errorMessage.Replace($"$({property})", replacement); } #endregion } diff --git a/test/RulesEngine.UnitTest/Issue519Test.cs b/test/RulesEngine.UnitTest/Issue519Test.cs new file mode 100644 index 00000000..3812e32d --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue519Test.cs @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue519Test + { + [Fact] + public async Task ExecuteActionWorkflowAsync_FailingRule_PopulatesExceptionMessageFromErrorMessage() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "input1 == \"expected\"", + ErrorMessage = "Input was $(input1), expected `expected`" + } + } + }; + var engine = new RulesEngine(new[] { workflow }); + + var actionResult = await engine.ExecuteActionWorkflowAsync("wf", "R", + new[] { RuleParameter.Create("input1", "actual-value") }); + + var ruleResult = actionResult.Results.Single(); + Assert.False(ruleResult.IsSuccess); + // Before the fix, ExceptionMessage was empty; after, it should contain the interpolated ErrorMessage. + Assert.False(string.IsNullOrEmpty(ruleResult.ExceptionMessage)); + Assert.Contains("actual-value", ruleResult.ExceptionMessage); + } + + [Fact] + public async Task ExecuteAllRulesAsync_BehavesTheSameForReference() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "input1 == \"expected\"", + ErrorMessage = "Input was $(input1), expected `expected`" + } + } + }; + var engine = new RulesEngine(new[] { workflow }); + var results = await engine.ExecuteAllRulesAsync("wf", "actual-value"); + + Assert.False(results[0].IsSuccess); + Assert.Contains("actual-value", results[0].ExceptionMessage); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue576Test.cs b/test/RulesEngine.UnitTest/Issue576Test.cs new file mode 100644 index 00000000..a2c15d99 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue576Test.cs @@ -0,0 +1,69 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Actions; +using RulesEngine.Models; +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue576Test + { + private class NoContextAction : ActionBase + { + public static bool WasRun; + public override ValueTask Run(ActionContext context, RuleParameter[] ruleParameters) + { + WasRun = true; + return new ValueTask("done"); + } + } + + [Fact] + public async Task CustomAction_WithNullContext_DoesNotThrow() + { + NoContextAction.WasRun = false; + + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "noctx", + Context = null + } + } + } + } + }; + var settings = new ReSettings + { + CustomActions = new Dictionary> + { + { "noctx", () => new NoContextAction() } + } + }; + var engine = new RulesEngine(new[] { workflow }, settings); + var results = await engine.ExecuteAllRulesAsync("wf", "x"); + + Assert.True(NoContextAction.WasRun, "Custom action should have run even with null Context"); + Assert.Null(results[0].ActionResult?.Exception); + } + + [Fact] + public void ActionContext_NullDictionary_DoesNotThrow() + { + var ex = Record.Exception(() => new ActionContext(null, null)); + Assert.Null(ex); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue581Test.cs b/test/RulesEngine.UnitTest/Issue581Test.cs new file mode 100644 index 00000000..f3eca822 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue581Test.cs @@ -0,0 +1,50 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue581Support + { + public class MyParam + { + public string Value1 { get; set; } + public string Value2 { get; set; } + } + } + + [ExcludeFromCodeCoverage] + public class Issue581Test + { + [Fact] + public async Task CustomParameterName_IsHonored() + { + var workflow = new Workflow + { + WorkflowName = "my_workflow", + Rules = new[] { + new Rule { + RuleName = "MatchesFabrikam", + Enabled = true, + RuleExpressionType = RuleExpressionType.LambdaExpression, + Expression = "myValue.Value1 == \"Fabrikam\"" + } + } + }; + + var input = new Issue581Support.MyParam { Value1 = "Fabrikam", Value2 = "x" }; + var rp = new RuleParameter("myValue", input); + var engine = new RulesEngine(new[] { workflow }); + + var results = await engine.ExecuteAllRulesAsync("my_workflow", new[] { rp }); + + Assert.True(results[0].IsSuccess, + $"Expected success. Got: {results[0].ExceptionMessage}"); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue590Test.cs b/test/RulesEngine.UnitTest/Issue590Test.cs new file mode 100644 index 00000000..147c6c7b --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue590Test.cs @@ -0,0 +1,65 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue590Test + { + public class FlakyInput + { + private int _counter; + private string _simpleProp; + public string SimpleProp + { + get + { + if (_counter++ == 0) + { + throw new ArgumentException("first-call-failure"); + } + return _simpleProp; + } + set { _simpleProp = value; } + } + } + + [Fact] + public async Task ExceptionMessage_FromPriorRunDoesNotLeakIntoNextSuccessfulRun() + { + var input = new FlakyInput { SimpleProp = "simpleProp" }; + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "CheckSimpleProp", + RuleExpressionType = RuleExpressionType.LambdaExpression, + Expression = "SimpleProp == \"simpleProp\"", + ErrorMessage = "should not leak" + } + } + }; + var settings = new ReSettings { UseFastExpressionCompiler = false }; + var engine = new RulesEngine(new[] { workflow }, settings); + + var firstRun = await engine.ExecuteAllRulesAsync("wf", input); + Assert.False(firstRun[0].IsSuccess); + Assert.False(string.IsNullOrEmpty(firstRun[0].ExceptionMessage)); + + var secondRun = await engine.ExecuteAllRulesAsync("wf", input); + Assert.True(secondRun[0].IsSuccess, + $"Second run should succeed. Got ExceptionMessage = `{secondRun[0].ExceptionMessage}`"); + Assert.True(string.IsNullOrEmpty(secondRun[0].ExceptionMessage), + $"Second run ExceptionMessage should be empty. Got `{secondRun[0].ExceptionMessage}`"); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue606Test.cs b/test/RulesEngine.UnitTest/Issue606Test.cs new file mode 100644 index 00000000..40e8f17b --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue606Test.cs @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue606Support + { + public class Line + { + public IReadOnlyList Modifiers { get; set; } = new List(); + } + } + + [ExcludeFromCodeCoverage] + public class Issue606Test + { + [Fact] + public async Task LambdaParameter_InAnyWithArrayLiteralContains_IsRecognized() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "ContainsModifier", + Expression = "line.Modifiers.Any(l => new [] {\"25\"}.Contains(l))" + } + } + }; + var input = new Issue606Support.Line { Modifiers = new List { "25", "30" } }; + var rp = new RuleParameter("line", input); + + var engine = new RulesEngine(new[] { workflow }); + var results = await engine.ExecuteAllRulesAsync("wf", new[] { rp }); + + Assert.True(results[0].IsSuccess, + $"Expected success. Got: {results[0].ExceptionMessage}"); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue608Test.cs b/test/RulesEngine.UnitTest/Issue608Test.cs new file mode 100644 index 00000000..8b988f5b --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue608Test.cs @@ -0,0 +1,81 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue608Support + { + public class AppData + { + public List Details { get; set; } = new List(); + } + public class Detail + { + public decimal? Amount { get; set; } + } + } + + [ExcludeFromCodeCoverage] + public class Issue608Test + { + private static Workflow BuildWorkflow() => new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "ChainedSum", + Expression = "Total > 0", + LocalParams = new List + { + new ScopedParam { Name = "Field1", Expression = "AppData.Details.Sum(l => l.Amount)" }, + new ScopedParam { Name = "Field2", Expression = "AppData.Details.Sum(l => l.Amount)" }, + new ScopedParam { Name = "Field3", Expression = "AppData.Details.Sum(l => l.Amount)" }, + new ScopedParam { Name = "Total", Expression = "Field1 + Field2 + Field3" } + } + } + } + }; + + private static Issue608Support.AppData BuildInput() => new Issue608Support.AppData + { + Details = new List + { + new Issue608Support.Detail { Amount = 1 }, + new Issue608Support.Detail { Amount = 2 }, + new Issue608Support.Detail { Amount = 3 }, + } + }; + + [Fact] + public async Task ChainedScopedParamSum_WithFastCompiler_Works() + { + var engine = new RulesEngine(new[] { BuildWorkflow() }, + new ReSettings { UseFastExpressionCompiler = true }); + + var rp = new RuleParameter("AppData", BuildInput()); + var results = await engine.ExecuteAllRulesAsync("wf", new[] { rp }); + + Assert.True(results[0].IsSuccess, + $"Expected success. Got: {results[0].ExceptionMessage}"); + } + + [Fact] + public async Task ChainedScopedParamSum_WithoutFastCompiler_StillWorks() + { + var engine = new RulesEngine(new[] { BuildWorkflow() }, + new ReSettings { UseFastExpressionCompiler = false }); + + var rp = new RuleParameter("AppData", BuildInput()); + var results = await engine.ExecuteAllRulesAsync("wf", new[] { rp }); + + Assert.True(results[0].IsSuccess); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue696Test.cs b/test/RulesEngine.UnitTest/Issue696Test.cs new file mode 100644 index 00000000..4609457e --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue696Test.cs @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue696Support + { + public class Nested + { + public Inner Inner { get; set; } + } + public class Inner + { + public string Name { get; set; } + } + } + + [ExcludeFromCodeCoverage] + public class Issue696Test + { + [Fact] + public async Task ErrorMessage_WithNestedDottedInterpolation_ResolvesRecursively() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "false", + ErrorMessage = "got $(input1.Inner.Name)" + } + } + }; + var input = new Issue696Support.Nested + { + Inner = new Issue696Support.Inner { Name = "deep-value" } + }; + var engine = new RulesEngine(new[] { workflow }); + var results = await engine.ExecuteAllRulesAsync("wf", + new[] { RuleParameter.Create("input1", input) }); + + Assert.Equal("got deep-value", results[0].ExceptionMessage); + } + + [Fact] + public async Task ErrorMessage_WithSingleLevelInterpolation_StillWorks() + { + // Regression guard for the existing one-level case. + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "false", + ErrorMessage = "got $(input1.Name)" + } + } + }; + var input = new Issue696Support.Inner { Name = "simple-value" }; + var engine = new RulesEngine(new[] { workflow }); + var results = await engine.ExecuteAllRulesAsync("wf", + new[] { RuleParameter.Create("input1", input) }); + + Assert.Contains("simple-value", results[0].ExceptionMessage); + } + } +}