From 3cb097bf794849e8766fb2e6080cf7a6ab59043d Mon Sep 17 00:00:00 2001 From: Yogesh Prajapati Date: Wed, 27 May 2026 16:17:45 +0100 Subject: [PATCH 1/2] Fix #717, #711, #714, #704, #624: bug bash batch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #624: ActionBase.ExecuteAndReturnResultAsync silently captured every exception into result.Exception regardless of EnableExceptionAsErrorMessage, so an action throwing inside Run() never propagated when the setting was false. Add ThrowIfActionExceptionShouldPropagate in RulesEngine to honor the contract documented on ReSettings. #711: OutputExpressionAction emitted a cryptic "Expression is missing an 'as' clause" when users wrote C#-style anonymous objects (`new { X = ... } as Result`). Detect the pattern and wrap the parse exception with a clear hint pointing at the Dynamic.Core syntax (`new (value as Name, ...)`). #714: Global params were re-evaluated for every rule in the workflow, so `Utils.FromDb(myInput)` ran N times per ExecuteAllRulesAsync call. Move the global-params compilation+evaluation to workflow scope: compile a single delegate at RegisterRule time, store it in RulesCache, evaluate once in ExecuteAllRuleByWorkflow and append the result as RuleParameters to each compiled rule. Preserve the existing per-rule error messages when global compilation or evaluation fails. ExecuteActionWorkflowAsync (which bypasses RulesCache) evaluates globals ad-hoc. #704: Utils.CreateAbstractClassType used only list[0] when generating the CLR type for a heterogeneous IList of ExpandoObject/Dictionary, so any property appearing only in later elements was dropped. Walk every element and union the schema; recursively merge nested dictionaries. #717: Methods declared to return `object` cannot have their result members accessed in Dynamic.Core expressions — the parser errors with "exists in type 'Object'" or "is not defined for the types 'System.Object'". We can't unbox at parse time, but the error message gave no clue what was wrong. Detect those patterns in LambdaExpressionBuilder and append a hint to change the method's return type to the concrete class. All 144 tests pass on net6.0 / net8.0 / net9.0 / net10.0. --- .../Actions/ExpressionOutputAction.cs | 20 +- .../LambdaExpressionBuilder.cs | 17 +- src/RulesEngine/HelperFunctions/Utils.cs | 107 ++++++++-- src/RulesEngine/RuleCompiler.cs | 8 +- src/RulesEngine/RulesCache.cs | 20 +- src/RulesEngine/RulesEngine.cs | 118 ++++++++++- test/RulesEngine.UnitTest/Issue624Test.cs | 187 ++++++++++++++++++ test/RulesEngine.UnitTest/Issue704Test.cs | 97 +++++++++ test/RulesEngine.UnitTest/Issue711Test.cs | 80 ++++++++ test/RulesEngine.UnitTest/Issue714Test.cs | 54 +++++ test/RulesEngine.UnitTest/Issue717Test.cs | 116 +++++++++++ 11 files changed, 787 insertions(+), 37 deletions(-) create mode 100644 test/RulesEngine.UnitTest/Issue624Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue704Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue711Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue714Test.cs create mode 100644 test/RulesEngine.UnitTest/Issue717Test.cs diff --git a/src/RulesEngine/Actions/ExpressionOutputAction.cs b/src/RulesEngine/Actions/ExpressionOutputAction.cs index 315b9965..b070975a 100644 --- a/src/RulesEngine/Actions/ExpressionOutputAction.cs +++ b/src/RulesEngine/Actions/ExpressionOutputAction.cs @@ -3,12 +3,18 @@ using RulesEngine.ExpressionBuilders; using RulesEngine.Models; +using System; +using System.Linq.Dynamic.Core.Exceptions; +using System.Text.RegularExpressions; using System.Threading.Tasks; namespace RulesEngine.Actions { public class OutputExpressionAction : ActionBase { + private static readonly Regex CSharpAnonymousObjectPattern = + new Regex(@"\bnew\s*\{", RegexOptions.Compiled); + private readonly RuleExpressionParser _ruleExpressionParser; public OutputExpressionAction(RuleExpressionParser ruleExpressionParser) @@ -19,7 +25,19 @@ public OutputExpressionAction(RuleExpressionParser ruleExpressionParser) public override ValueTask Run(ActionContext context, RuleParameter[] ruleParameters) { var expression = context.GetContext("expression"); - return new ValueTask(_ruleExpressionParser.Evaluate(expression, ruleParameters)); + try + { + return new ValueTask(_ruleExpressionParser.Evaluate(expression, ruleParameters)); + } + catch (ParseException ex) when (CSharpAnonymousObjectPattern.IsMatch(expression ?? string.Empty)) + { + throw new ParseException( + "OutputExpression failed to parse. It looks like the expression uses C#-style anonymous-object syntax " + + "(`new { Name = value, ... }`), which is not supported by System.Linq.Dynamic.Core. " + + "Use the Dynamic.Core form instead: `new (value as Name, ...)` — parentheses, and each field needs an `as Alias`. " + + "Original parser error: " + ex.Message, + ex.Position); + } } } } diff --git a/src/RulesEngine/ExpressionBuilders/LambdaExpressionBuilder.cs b/src/RulesEngine/ExpressionBuilders/LambdaExpressionBuilder.cs index b35961bb..e476eabb 100644 --- a/src/RulesEngine/ExpressionBuilders/LambdaExpressionBuilder.cs +++ b/src/RulesEngine/ExpressionBuilders/LambdaExpressionBuilder.cs @@ -34,11 +34,24 @@ internal override RuleFunc BuildDelegateForRule(Rule rule, RuleP { Helpers.HandleRuleException(ex,rule,_reSettings); - var exceptionMessage = Helpers.GetExceptionMessage($"Exception while parsing expression `{rule?.Expression}` - {ex.Message}", + var detail = ex.Message; + if (detail != null + && (detail.Contains("exists in type 'Object'") + || detail.Contains("'System.Object'")) + && (rule?.Expression?.Contains('(') == true)) + { + // Dynamic.Core can only resolve members and operators against a static return type. + // If a custom/static method's declared return type is `object`, member access or + // operator usage on its result fails. See #717. + detail += " (Hint: a method called in this expression appears to have an `object` return type. " + + "Change its return type to the concrete class — Dynamic.Core cannot resolve members or operators on `object`.)"; + } + + var exceptionMessage = Helpers.GetExceptionMessage($"Exception while parsing expression `{rule?.Expression}` - {detail}", _reSettings); bool func(object[] param) => false; - + return Helpers.ToResultTree(_reSettings, rule, null,func, exceptionMessage); } } diff --git a/src/RulesEngine/HelperFunctions/Utils.cs b/src/RulesEngine/HelperFunctions/Utils.cs index 98c0953a..cf48bb8f 100644 --- a/src/RulesEngine/HelperFunctions/Utils.cs +++ b/src/RulesEngine/HelperFunctions/Utils.cs @@ -55,16 +55,7 @@ public static Type CreateAbstractClassType(dynamic input) Type value; if (expando.Value is IList list) { - if (list.Count == 0) - { - value = typeof(List); - } - else - { - var internalType = CreateAbstractClassType(list[0]); - value = new List().Cast(internalType).ToList(internalType).GetType(); - } - + value = BuildListType(list); } else { @@ -77,6 +68,90 @@ public static Type CreateAbstractClassType(dynamic input) return type; } + // 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. + private static Type BuildListType(IList list) + { + if (list.Count == 0) + { + return typeof(List); + } + + var firstElement = list[0]; + if (firstElement is ExpandoObject || firstElement is IDictionary) + { + var merged = MergeListElementSchemas(list); + var internalType = CreateAbstractClassTypeFromDictionary(merged); + return new List().Cast(internalType).ToList(internalType).GetType(); + } + + // Non-schema-like element: fall back to first-element type as before. + var legacyType = CreateAbstractClassType(firstElement); + return new List().Cast(legacyType).ToList(legacyType).GetType(); + } + + private static IDictionary MergeListElementSchemas(IList list) + { + var merged = new Dictionary(); + foreach (var elem in list) + { + if (elem is not IDictionary elemDict) + { + continue; + } + foreach (var kvp in elemDict) + { + if (!merged.TryGetValue(kvp.Key, out var existing)) + { + merged[kvp.Key] = kvp.Value; + continue; + } + + if (existing is IDictionary existingDict && kvp.Value is IDictionary newDict) + { + merged[kvp.Key] = MergeTwoDictionaries(existingDict, newDict); + } + else if (existing is IList existingList && kvp.Value is IList newList) + { + var combined = new List(); + foreach (var e in existingList) combined.Add(e); + foreach (var e in newList) combined.Add(e); + merged[kvp.Key] = combined; + } + else if (existing == null && kvp.Value != null) + { + merged[kvp.Key] = kvp.Value; + } + // Else keep existing (first non-null wins on type conflict). + } + } + return merged; + } + + private static IDictionary MergeTwoDictionaries(IDictionary a, IDictionary b) + { + var merged = new Dictionary(); + foreach (var kvp in a) merged[kvp.Key] = kvp.Value; + foreach (var kvp in b) + { + if (!merged.TryGetValue(kvp.Key, out var existing)) + { + merged[kvp.Key] = kvp.Value; + continue; + } + if (existing is IDictionary ea && kvp.Value is IDictionary eb) + { + merged[kvp.Key] = MergeTwoDictionaries(ea, eb); + } + else if (existing == null && kvp.Value != null) + { + merged[kvp.Key] = kvp.Value; + } + } + return merged; + } + public static object CreateObject(Type type, dynamic input) { if (input is not ExpandoObject expandoObject) @@ -152,17 +227,7 @@ private static Type CreateAbstractClassTypeFromDictionary(IDictionary); - } - else - { - var internalType = list[0] is IDictionary innerDict - ? CreateAbstractClassTypeFromDictionary(innerDict) - : (list[0] is ExpandoObject ? CreateAbstractClassType(list[0]) : list[0]?.GetType() ?? typeof(object)); - valueType = new List().Cast(internalType).ToList(internalType).GetType(); - } + valueType = BuildListType(list); } else { diff --git a/src/RulesEngine/RuleCompiler.cs b/src/RulesEngine/RuleCompiler.cs index 4a036039..cc7913b0 100644 --- a/src/RulesEngine/RuleCompiler.cs +++ b/src/RulesEngine/RuleCompiler.cs @@ -59,10 +59,10 @@ internal RuleFunc CompileRule(Rule rule, RuleExpressionType rule var globalParamExp = globalParams.Value; var extendedRuleParams = ruleParams.Concat(globalParamExp.Select(c => new RuleParameter(c.ParameterExpression.Name,c.ParameterExpression.Type))) .ToArray(); - var ruleExpression = GetDelegateForRule(rule, extendedRuleParams); - - - return GetWrappedRuleFunc(rule,ruleExpression,ruleParams,globalParamExp); + // Note: globals are no longer evaluated here per rule. The caller is expected + // to evaluate workflow-level globals once and pass them as extra RuleParameters + // when invoking the returned delegate. See #714. + return GetDelegateForRule(rule, extendedRuleParams); } catch (Exception ex) { diff --git a/src/RulesEngine/RulesCache.cs b/src/RulesEngine/RulesCache.cs index 9f0bc30a..968d7b80 100644 --- a/src/RulesEngine/RulesCache.cs +++ b/src/RulesEngine/RulesCache.cs @@ -16,6 +16,9 @@ internal class RulesCache /// The compile rules private readonly MemCache _compileRules; + /// Per-workflow compiled delegate that evaluates all global params once. + private readonly MemCache _compiledGlobalParams; + /// The workflow rules private readonly ConcurrentDictionary _workflow = new ConcurrentDictionary(); @@ -23,6 +26,19 @@ internal class RulesCache public RulesCache(ReSettings reSettings) { _compileRules = new MemCache(reSettings.CacheConfig); + _compiledGlobalParams = new MemCache(reSettings.CacheConfig); + } + + /// Adds or updates the workflow-level global-params delegate. + public void AddOrUpdateGlobalParamsDelegate(string compiledRuleKey, Func> globalParamsDelegate) + { + _compiledGlobalParams.Set(compiledRuleKey, globalParamsDelegate); + } + + /// Gets the workflow-level global-params delegate, or null if the workflow has no globals. + public Func> GetGlobalParamsDelegate(string compiledRuleKey) + { + return _compiledGlobalParams.Get>>(compiledRuleKey); } @@ -81,6 +97,7 @@ public void Clear() { _workflow.Clear(); _compileRules.Clear(); + _compiledGlobalParams.Clear(); } /// Gets the work flow rules. @@ -133,10 +150,11 @@ public void Remove(string workflowName) { if (_workflow.TryRemove(workflowName, out var workflowObj)) { - var compiledKeysToRemove = _compileRules.GetKeys().Where(key => key.StartsWith(workflowName)); + var compiledKeysToRemove = _compileRules.GetKeys().Where(key => key.StartsWith(workflowName)).ToList(); foreach (var key in compiledKeysToRemove) { _compileRules.Remove(key); + _compiledGlobalParams.Remove(key); } } } diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index f50a26f0..dbe49a5f 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -123,14 +123,51 @@ private async ValueTask ExecuteActionAsync(IEnumerable ruleResul Output = actionResult.Output, Exception = actionResult.Exception }; + ThrowIfActionExceptionShouldPropagate(actionResult, ruleResult); } } public async ValueTask ExecuteActionWorkflowAsync(string workflowName, string ruleName, RuleParameter[] ruleParameters) { var compiledRule = CompileRule(workflowName, ruleName, ruleParameters); - var resultTree = compiledRule(ruleParameters); - return await ExecuteActionForRuleResult(resultTree, true); + var extendedRuleParameters = EvaluateGlobalsAdHoc(workflowName, ruleParameters); + var resultTree = compiledRule(extendedRuleParameters); + var actionResult = await ExecuteActionForRuleResult(resultTree, true); + ThrowIfActionExceptionShouldPropagate(actionResult, resultTree); + return actionResult; + } + + private RuleParameter[] EvaluateGlobalsAdHoc(string workflowName, RuleParameter[] ruleParameters) + { + var workflow = _rulesCache.GetWorkflow(workflowName); + if (workflow?.GlobalParams == null || !workflow.GlobalParams.Any()) + { + return ruleParameters; + } + var globalParamValues = _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParameters); + if (globalParamValues.Length == 0) + { + return ruleParameters; + } + var globalParamsDelegate = _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParameters, globalParamValues); + var inputs = ruleParameters.Select(c => c.Value).ToArray(); + var evaluated = globalParamsDelegate(inputs); + var globals = evaluated.Select(kv => new RuleParameter(kv.Key, kv.Value)); + return ruleParameters.Concat(globals).ToArray(); + } + + private void ThrowIfActionExceptionShouldPropagate(ActionRuleResult actionResult, RuleResultTree resultTree) + { + if (actionResult?.Exception == null) + { + return; + } + if (_reSettings.IgnoreException || _reSettings.EnableExceptionAsErrorMessage) + { + return; + } + actionResult.Exception.Data[nameof(Rule.RuleName)] = resultTree?.Rule?.RuleName; + throw actionResult.Exception; } private async ValueTask ExecuteActionForRuleResult(RuleResultTree resultTree, bool includeRuleResults = false) @@ -300,11 +337,33 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams _reSettings.CustomTypes = collector.ToArray(); } - // add separate compilation for global params + // Compile global params ONCE per workflow registration. The resulting delegate is + // invoked once per ExecuteAllRulesAsync call (in ExecuteAllRuleByWorkflow) and its + // results passed as extra RuleParameters to each rule. See #714. + RuleExpressionParameter[] globalParamValues; + try + { + globalParamValues = _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParams); + } + catch (Exception ex) + { + // Mirror the legacy per-rule error reporting when global-param compilation fails. + foreach (var rule in workflow.Rules.Where(c => c.Enabled)) + { + var msg = $"Error while compiling rule `{rule.RuleName}`: {ex.Message}"; + dictFunc.Add(rule.RuleName, Helpers.ToRuleExceptionResult(_reSettings, rule, new RuleException(msg, ex))); + } + _rulesCache.AddOrUpdateCompiledRule(compileRulesKey, dictFunc); + return true; + } - var globalParamExp = new Lazy( - () => _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParams) - ); + var globalParamExp = new Lazy(() => globalParamValues); + + if (globalParamValues.Length > 0) + { + var globalParamsDelegate = _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParams, globalParamValues); + _rulesCache.AddOrUpdateGlobalParamsDelegate(compileRulesKey, globalParamsDelegate); + } foreach (var rule in workflow.Rules.Where(c => c.Enabled)) { @@ -378,9 +437,39 @@ private List ExecuteAllRuleByWorkflow(string workflowName, RuleP { var result = new List(); var compiledRulesCacheKey = GetCompiledRulesKey(workflowName, ruleParameters); - foreach (var compiledRule in _rulesCache.GetCompiledRules(compiledRulesCacheKey)?.Values) + var compiledRules = _rulesCache.GetCompiledRules(compiledRulesCacheKey); + + RuleParameter[] extendedRuleParameters; + Exception globalEvaluationException = null; + try + { + extendedRuleParameters = ApplyGlobalParams(compiledRulesCacheKey, ruleParameters); + } + catch (Exception ex) + { + globalEvaluationException = ex; + extendedRuleParameters = ruleParameters; + } + + var ruleByName = new Dictionary(); + foreach (var rule in _rulesCache.GetWorkflow(workflowName)?.Rules?.Where(c => c.Enabled) ?? Enumerable.Empty()) + { + ruleByName[rule.RuleName] = rule; + } + + foreach (var compiledRule in compiledRules) { - var resultTree = compiledRule(ruleParameters); + RuleResultTree resultTree; + if (globalEvaluationException != null && ruleByName != null && ruleByName.TryGetValue(compiledRule.Key, out var rule)) + { + var msg = $"Error while executing scoped params for rule `{rule.RuleName}` - {globalEvaluationException.Message}"; + var errFn = Helpers.ToRuleExceptionResult(_reSettings, rule, new RuleException(msg, globalEvaluationException)); + resultTree = errFn(ruleParameters); + } + else + { + resultTree = compiledRule.Value(extendedRuleParameters); + } result.Add(resultTree); } @@ -388,6 +477,19 @@ private List ExecuteAllRuleByWorkflow(string workflowName, RuleP return result; } + private RuleParameter[] ApplyGlobalParams(string compiledRulesCacheKey, RuleParameter[] ruleParameters) + { + var globalParamsDelegate = _rulesCache.GetGlobalParamsDelegate(compiledRulesCacheKey); + if (globalParamsDelegate == null) + { + return ruleParameters; + } + var inputs = ruleParameters.Select(c => c.Value).ToArray(); + var evaluated = globalParamsDelegate(inputs); + var globals = evaluated.Select(kv => new RuleParameter(kv.Key, kv.Value)); + return ruleParameters.Concat(globals).ToArray(); + } + private string GetCompiledRulesKey(string workflowName, RuleParameter[] ruleParams) { var ruleParamsKey = string.Join("-", ruleParams.Select(c => $"{c.Name}_{c.Type.Name}")); diff --git a/test/RulesEngine.UnitTest/Issue624Test.cs b/test/RulesEngine.UnitTest/Issue624Test.cs new file mode 100644 index 00000000..65f87117 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue624Test.cs @@ -0,0 +1,187 @@ +// 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 Issue624Test + { + private class ThrowingAction : ActionBase + { + public override ValueTask Run(ActionContext context, RuleParameter[] ruleParameters) + { + throw new InvalidOperationException("boom from custom action"); + } + } + + [Fact] + public async Task Repro_InvalidExpression_WithEnableExceptionAsErrorMessageFalse_ShouldThrow() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "BadRule", + Expression = "input1.NotARealProperty.Foo == 1" + } + } + }; + + var engine = new RulesEngine( + new[] { workflow }, + new ReSettings { EnableExceptionAsErrorMessage = false }); + + await Assert.ThrowsAnyAsync(async () => + await engine.ExecuteAllRulesAsync("wf", "hello")); + } + + [Fact] + public async Task Repro_RuntimeException_WithEnableExceptionAsErrorMessageFalse_ShouldThrow() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "DivideByZero", + Expression = "(1 / input1) == 1" + } + } + }; + + var engine = new RulesEngine( + new[] { workflow }, + new ReSettings { EnableExceptionAsErrorMessage = false }); + + await Assert.ThrowsAnyAsync(async () => + await engine.ExecuteAllRulesAsync("wf", 0)); + } + + [Fact] + public async Task Repro_InvalidExpression_ExecuteActionWorkflowAsync_WithFlagFalse_ShouldThrow() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "BadRule", + Expression = "input1.NotARealProperty.Foo == 1" + } + } + }; + + var engine = new RulesEngine( + new[] { workflow }, + new ReSettings { EnableExceptionAsErrorMessage = false }); + + await Assert.ThrowsAnyAsync(async () => + await engine.ExecuteActionWorkflowAsync("wf", "BadRule", + new[] { RuleParameter.Create("input1", "hello") })); + } + + [Fact] + public async Task CustomActionThrows_WithEnableExceptionAsErrorMessageFalse_ShouldPropagate() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "RuleWithThrowingAction", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "throwing", + Context = new Dictionary() + } + } + } + } + }; + + var settings = new ReSettings + { + EnableExceptionAsErrorMessage = false, + CustomActions = new Dictionary> + { + { "throwing", () => new ThrowingAction() } + } + }; + + var engine = new RulesEngine(new[] { workflow }, settings); + + await Assert.ThrowsAnyAsync(async () => + await engine.ExecuteAllRulesAsync("wf", "x")); + } + + [Fact] + public async Task CustomActionThrows_WithEnableExceptionAsErrorMessageTrue_ShouldCaptureInResult() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "RuleWithThrowingAction", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "throwing", + Context = new Dictionary() + } + } + } + } + }; + + var settings = new ReSettings + { + EnableExceptionAsErrorMessage = true, + CustomActions = new Dictionary> + { + { "throwing", () => new ThrowingAction() } + } + }; + + var engine = new RulesEngine(new[] { workflow }, settings); + + var results = await engine.ExecuteAllRulesAsync("wf", "x"); + Assert.Single(results); + Assert.NotNull(results[0].ActionResult); + Assert.NotNull(results[0].ActionResult.Exception); + } + + [Fact] + public async Task Default_EnableExceptionAsErrorMessageTrue_ShouldNotThrow_ReportsAsErrorMessage() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "BadRule", + Expression = "input1.NotARealProperty.Foo == 1" + } + } + }; + + // Default settings: EnableExceptionAsErrorMessage = true + var engine = new RulesEngine(new[] { workflow }); + + var results = await engine.ExecuteAllRulesAsync("wf", "hello"); + Assert.Single(results); + Assert.False(results[0].IsSuccess); + Assert.False(string.IsNullOrEmpty(results[0].ExceptionMessage)); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue704Test.cs b/test/RulesEngine.UnitTest/Issue704Test.cs new file mode 100644 index 00000000..eb026a8a --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue704Test.cs @@ -0,0 +1,97 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.HelperFunctions; +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Dynamic; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue704Test + { + [Fact] + public void GetTypedObject_ListOfExpandoWithDifferentProperties_KeepsLaterElementsProperties() + { + // Element 0 has properties { a, b }, element 1 has { a, b, c }. + // Expected: the generated type includes 'c', and element 1's 'c' value survives. + IDictionary e0 = new ExpandoObject(); + e0["a"] = 1; + e0["b"] = "hello"; + + IDictionary e1 = new ExpandoObject(); + e1["a"] = 2; + e1["b"] = "world"; + e1["c"] = 42; + + IDictionary root = new ExpandoObject(); + root["items"] = new List { e0, e1 }; + + var typed = Utils.GetTypedObject(root); + var itemsProp = typed.GetType().GetProperty("items"); + Assert.NotNull(itemsProp); + + var itemsList = (System.Collections.IList)itemsProp.GetValue(typed); + Assert.Equal(2, itemsList.Count); + + var elemType = itemsList[0].GetType(); + Assert.NotNull(elemType.GetProperty("a")); + Assert.NotNull(elemType.GetProperty("b")); + // The key assertion: 'c' must be in the unified type + Assert.NotNull(elemType.GetProperty("c")); + + // And element 1 must actually carry the 'c' value + var elem1C = elemType.GetProperty("c").GetValue(itemsList[1]); + Assert.Equal(42, elem1C); + } + + [Fact] + public void GetTypedObject_NestedListPropertiesAreUnioned() + { + // records[0].product.details has { type, sku }; records[1].product.details has { type, sku, loc } + IDictionary details0 = new ExpandoObject(); + details0["type"] = "electronic"; + details0["sku"] = "123"; + + IDictionary product0 = new ExpandoObject(); + product0["details"] = details0; + + IDictionary record0 = new ExpandoObject(); + record0["product"] = product0; + + IDictionary details1 = new ExpandoObject(); + details1["type"] = "electronic"; + details1["sku"] = "45"; + details1["loc"] = "TR"; + + IDictionary product1 = new ExpandoObject(); + product1["details"] = details1; + + IDictionary record1 = new ExpandoObject(); + record1["product"] = product1; + + IDictionary root = new ExpandoObject(); + root["records"] = new List { record0, record1 }; + + var typed = Utils.GetTypedObject(root); + var records = (System.Collections.IList)typed.GetType().GetProperty("records").GetValue(typed); + + var detailsType = records[1].GetType() + .GetProperty("product").PropertyType + .GetProperty("details").PropertyType; + + Assert.NotNull(detailsType.GetProperty("type")); + Assert.NotNull(detailsType.GetProperty("sku")); + Assert.NotNull(detailsType.GetProperty("loc")); + + var product = records[1].GetType().GetProperty("product").GetValue(records[1]); + var details = product.GetType().GetProperty("details").GetValue(product); + Assert.Equal("TR", details.GetType().GetProperty("loc").GetValue(details)); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue711Test.cs b/test/RulesEngine.UnitTest/Issue711Test.cs new file mode 100644 index 00000000..014ba7eb --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue711Test.cs @@ -0,0 +1,80 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue711Test + { + [Fact] + public async Task OutputExpression_AnonymousObjectWithCSharpStyleSyntax_FailsWithDynamicCoreError() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "OutputExpression", + Context = new Dictionary { + { "expression", "new { State = input1, CalculatedValue = 42 } as Result" } + } + } + } + } + } + }; + + var engine = new RulesEngine(new[] { workflow }); + + var result = await engine.ExecuteActionWorkflowAsync("wf", "R", + new[] { RuleParameter.Create("input1", "CA") }); + + Assert.NotNull(result.Exception); + Assert.Contains("Dynamic.Core", result.Exception.Message); + Assert.Contains("as Alias", result.Exception.Message); + } + + [Fact] + public async Task OutputExpression_AnonymousObjectWithDynamicCoreSyntax_Works() + { + // Dynamic.Core syntax: parens, each field needs "as alias" + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R", + Expression = "true", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "OutputExpression", + Context = new Dictionary { + { "expression", "new (input1 as State, 42 as CalculatedValue)" } + } + } + } + } + } + }; + + var engine = new RulesEngine(new[] { workflow }); + + var result = await engine.ExecuteActionWorkflowAsync("wf", "R", + new[] { RuleParameter.Create("input1", "CA") }); + + Assert.Null(result.Exception); + Assert.NotNull(result.Output); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue714Test.cs b/test/RulesEngine.UnitTest/Issue714Test.cs new file mode 100644 index 00000000..1d4e0ada --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue714Test.cs @@ -0,0 +1,54 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Diagnostics.CodeAnalysis; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public static class Issue714Counter + { + private static int _count; + public static int CallCount => _count; + public static void Reset() => Interlocked.Exchange(ref _count, 0); + public static string FromDb(string s) + { + Interlocked.Increment(ref _count); + return s; + } + } + + [ExcludeFromCodeCoverage] + public class Issue714Test + { + [Fact] + public async Task GlobalParam_WithMultipleRules_IsEvaluatedOnce() + { + Issue714Counter.Reset(); + + var workflow = new Workflow + { + WorkflowName = "wf", + GlobalParams = new[] { + new ScopedParam { Name = "myglobal1", Expression = "Issue714Counter.FromDb(input1)" } + }, + Rules = new[] { + new Rule { RuleName = "r1", Expression = "myglobal1 == \"hello\"" }, + new Rule { RuleName = "r2", Expression = "input1.ToLower() == myglobal1" }, + new Rule { RuleName = "r3", Expression = "myglobal1.Length == 5" } + } + }; + + var engine = new RulesEngine(new[] { workflow }, + new ReSettings { CustomTypes = new[] { typeof(Issue714Counter) } }); + + await engine.ExecuteAllRulesAsync("wf", "hello"); + + Assert.Equal(1, Issue714Counter.CallCount); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue717Test.cs b/test/RulesEngine.UnitTest/Issue717Test.cs new file mode 100644 index 00000000..2db732a3 --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue717Test.cs @@ -0,0 +1,116 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System; +using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public class Issue717TestSupport + { + public class SimpleInput + { + public int Value { get; set; } + public string Name { get; set; } + } + + public class ReturnTypeForIssue717 + { + public int val { get; set; } + } + + public static class FunctionsForIssue717 + { + public static object CheckValueAsObject(SimpleInput input) + { + if (input != null && input.Value > 5) + { + return new ReturnTypeForIssue717 { val = 1 }; + } + return new ReturnTypeForIssue717 { val = 0 }; + } + + public static ReturnTypeForIssue717 CheckValueTyped(SimpleInput input) + { + if (input != null && input.Value > 5) + { + return new ReturnTypeForIssue717 { val = 1 }; + } + return new ReturnTypeForIssue717 { val = 0 }; + } + } + } + + [ExcludeFromCodeCoverage] + public class Issue717Test + { + [Fact] + public async Task CustomMethodReturningTypedClass_Works() + { + // Documenting the working scenario as a regression guard for the workaround. + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R1", + Expression = "FunctionsForIssue717.CheckValueTyped(myObj).val == 1" + } + } + }; + var settings = new ReSettings + { + CustomTypes = new[] + { + typeof(Issue717TestSupport.SimpleInput), + typeof(Issue717TestSupport.ReturnTypeForIssue717), + typeof(Issue717TestSupport.FunctionsForIssue717) + } + }; + var engine = new RulesEngine(new[] { workflow }, settings); + var input = new Issue717TestSupport.SimpleInput { Value = 10, Name = "x" }; + var results = await engine.ExecuteAllRulesAsync("wf", + new[] { RuleParameter.Create("myObj", input) }); + Assert.True(results[0].IsSuccess); + } + + [Fact] + public async Task CustomMethodReturningObject_ProducesHelpfulErrorMessage() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R1", + Expression = "FunctionsForIssue717.CheckValueAsObject(myObj).val == 1" + } + } + }; + var settings = new ReSettings + { + CustomTypes = new[] + { + typeof(Issue717TestSupport.SimpleInput), + typeof(Issue717TestSupport.ReturnTypeForIssue717), + typeof(Issue717TestSupport.FunctionsForIssue717) + } + }; + var engine = new RulesEngine(new[] { workflow }, settings); + var input = new Issue717TestSupport.SimpleInput { Value = 10, Name = "x" }; + var results = await engine.ExecuteAllRulesAsync("wf", + new[] { RuleParameter.Create("myObj", input) }); + + Assert.False(results[0].IsSuccess); + // Error must explicitly mention the return type / typed-return guidance. + Assert.True( + results[0].ExceptionMessage.IndexOf("return type", StringComparison.OrdinalIgnoreCase) >= 0, + $"Expected helpful hint about return type. Got: {results[0].ExceptionMessage}"); + } + } +} From dbaf2b9662ece7f809c8560f5ac473cdd3b0d7d4 Mon Sep 17 00:00:00 2001 From: Yogesh Prajapati Date: Wed, 27 May 2026 16:23:39 +0100 Subject: [PATCH 2/2] Deduplicate helpers introduced in the batch fix - RulesEngine.cs: ApplyGlobalParams and EvaluateGlobalsAdHoc both built RuleParameters from a globals delegate and concatenated them with the user's inputs. Extracted AppendGlobals (delegate-invoke + concat tail) and CompileGlobalParamsDelegate (GetRuleExpressionParameters + CompileScopedParams). - Utils.cs: MergeListElementSchemas and MergeTwoDictionaries shared the same pair-merge logic. Replaced both with MergeDictionaries (n-ary fold) + MergeValues (handles dict/dict, list/list, and first-non-null fallback). The latter also makes nested list-concatenation consistent across all call sites. No behavior change. All 144 tests still pass on net6/8/9/10. --- src/RulesEngine/HelperFunctions/Utils.cs | 69 ++++++++---------------- src/RulesEngine/RulesEngine.cs | 33 ++++++++---- 2 files changed, 44 insertions(+), 58 deletions(-) diff --git a/src/RulesEngine/HelperFunctions/Utils.cs b/src/RulesEngine/HelperFunctions/Utils.cs index cf48bb8f..b13af66d 100644 --- a/src/RulesEngine/HelperFunctions/Utils.cs +++ b/src/RulesEngine/HelperFunctions/Utils.cs @@ -81,7 +81,7 @@ private static Type BuildListType(IList list) var firstElement = list[0]; if (firstElement is ExpandoObject || firstElement is IDictionary) { - var merged = MergeListElementSchemas(list); + var merged = MergeDictionaries(list.OfType>()); var internalType = CreateAbstractClassTypeFromDictionary(merged); return new List().Cast(internalType).ToList(internalType).GetType(); } @@ -91,65 +91,38 @@ private static Type BuildListType(IList list) return new List().Cast(legacyType).ToList(legacyType).GetType(); } - private static IDictionary MergeListElementSchemas(IList list) + // Unions schemas from any number of dict-like inputs. Used both to merge sibling + // elements of a heterogeneous list (#704) and to merge nested dicts recursively. + private static IDictionary MergeDictionaries(IEnumerable> dictionaries) { var merged = new Dictionary(); - foreach (var elem in list) + foreach (var dict in dictionaries) { - if (elem is not IDictionary elemDict) + foreach (var kvp in dict) { - continue; - } - foreach (var kvp in elemDict) - { - if (!merged.TryGetValue(kvp.Key, out var existing)) - { - merged[kvp.Key] = kvp.Value; - continue; - } - - if (existing is IDictionary existingDict && kvp.Value is IDictionary newDict) - { - merged[kvp.Key] = MergeTwoDictionaries(existingDict, newDict); - } - else if (existing is IList existingList && kvp.Value is IList newList) - { - var combined = new List(); - foreach (var e in existingList) combined.Add(e); - foreach (var e in newList) combined.Add(e); - merged[kvp.Key] = combined; - } - else if (existing == null && kvp.Value != null) - { - merged[kvp.Key] = kvp.Value; - } - // Else keep existing (first non-null wins on type conflict). + merged[kvp.Key] = merged.TryGetValue(kvp.Key, out var existing) + ? MergeValues(existing, kvp.Value) + : kvp.Value; } } return merged; } - private static IDictionary MergeTwoDictionaries(IDictionary a, IDictionary b) + private static object MergeValues(object existing, object incoming) { - var merged = new Dictionary(); - foreach (var kvp in a) merged[kvp.Key] = kvp.Value; - foreach (var kvp in b) + if (existing is IDictionary a && incoming is IDictionary b) { - if (!merged.TryGetValue(kvp.Key, out var existing)) - { - merged[kvp.Key] = kvp.Value; - continue; - } - if (existing is IDictionary ea && kvp.Value is IDictionary eb) - { - merged[kvp.Key] = MergeTwoDictionaries(ea, eb); - } - else if (existing == null && kvp.Value != null) - { - merged[kvp.Key] = kvp.Value; - } + return MergeDictionaries(new[] { a, b }); } - return merged; + if (existing is IList la && incoming is IList lb) + { + var combined = new List(); + foreach (var e in la) combined.Add(e); + foreach (var e in lb) combined.Add(e); + return combined; + } + // First non-null wins on type conflict. + return existing ?? incoming; } public static object CreateObject(Type type, dynamic input) diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index dbe49a5f..3c125673 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -144,12 +144,33 @@ private RuleParameter[] EvaluateGlobalsAdHoc(string workflowName, RuleParameter[ { return ruleParameters; } + var globalParamsDelegate = CompileGlobalParamsDelegate(workflow, ruleParameters); + return AppendGlobals(ruleParameters, globalParamsDelegate); + } + + // Compiles a single delegate that evaluates all of a workflow's GlobalParams. + // Returns null if the workflow declares no globals. + private Func> CompileGlobalParamsDelegate(Workflow workflow, RuleParameter[] ruleParameters) + { + if (workflow?.GlobalParams == null || !workflow.GlobalParams.Any()) + { + return null; + } var globalParamValues = _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParameters); if (globalParamValues.Length == 0) + { + return null; + } + return _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParameters, globalParamValues); + } + + // Invokes the supplied globals delegate (if any) and appends the results as RuleParameters. + private static RuleParameter[] AppendGlobals(RuleParameter[] ruleParameters, Func> globalParamsDelegate) + { + if (globalParamsDelegate == null) { return ruleParameters; } - var globalParamsDelegate = _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParameters, globalParamValues); var inputs = ruleParameters.Select(c => c.Value).ToArray(); var evaluated = globalParamsDelegate(inputs); var globals = evaluated.Select(kv => new RuleParameter(kv.Key, kv.Value)); @@ -479,15 +500,7 @@ private List ExecuteAllRuleByWorkflow(string workflowName, RuleP private RuleParameter[] ApplyGlobalParams(string compiledRulesCacheKey, RuleParameter[] ruleParameters) { - var globalParamsDelegate = _rulesCache.GetGlobalParamsDelegate(compiledRulesCacheKey); - if (globalParamsDelegate == null) - { - return ruleParameters; - } - var inputs = ruleParameters.Select(c => c.Value).ToArray(); - var evaluated = globalParamsDelegate(inputs); - var globals = evaluated.Select(kv => new RuleParameter(kv.Key, kv.Value)); - return ruleParameters.Concat(globals).ToArray(); + return AppendGlobals(ruleParameters, _rulesCache.GetGlobalParamsDelegate(compiledRulesCacheKey)); } private string GetCompiledRulesKey(string workflowName, RuleParameter[] ruleParams)