diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index b1627063..74d6a307 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -129,8 +129,26 @@ private async ValueTask ExecuteActionAsync(IEnumerable ruleResul public async ValueTask ExecuteActionWorkflowAsync(string workflowName, string ruleName, RuleParameter[] ruleParameters) { - var compiledRule = CompileRule(workflowName, ruleName, ruleParameters); - var extendedRuleParameters = EvaluateGlobalsAdHoc(workflowName, ruleParameters); + // Sort to match the cache-key convention used by ExecuteAllRulesAsync. + var sortedRuleParams = ruleParameters.ToList(); + sortedRuleParams.Sort((a, b) => string.Compare(a.Name, b.Name)); + var sortedArr = sortedRuleParams.ToArray(); + + // Compile the whole workflow once and reuse — was previously recompiled every call, + // which was the hot path the reporter of #471 saw. + if (!RegisterRule(workflowName, sortedArr)) + { + throw new ArgumentException($"Rule config file is not present for the {workflowName} workflow"); + } + + var compiledRulesCacheKey = GetCompiledRulesKey(workflowName, sortedArr); + var compiledRules = _rulesCache.GetCompiledRules(compiledRulesCacheKey); + if (compiledRules == null || !compiledRules.TryGetValue(ruleName, out var compiledRule)) + { + throw new ArgumentException($"Workflow `{workflowName}` does not contain any rule named `{ruleName}`"); + } + + var extendedRuleParameters = ApplyGlobalParams(compiledRulesCacheKey, sortedArr); var resultTree = compiledRule(extendedRuleParameters); // Mirror ExecuteAllRulesAsync's behavior: format the per-rule ErrorMessage template // into ExceptionMessage before any action runs / before returning. See #519. @@ -140,33 +158,6 @@ public async ValueTask ExecuteActionWorkflowAsync(string workf return actionResult; } - private RuleParameter[] EvaluateGlobalsAdHoc(string workflowName, RuleParameter[] ruleParameters) - { - var workflow = _rulesCache.GetWorkflow(workflowName); - if (workflow?.GlobalParams == null || !workflow.GlobalParams.Any()) - { - 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) { @@ -404,24 +395,6 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams } - private RuleFunc CompileRule(string workflowName, string ruleName, RuleParameter[] ruleParameters) - { - var workflow = _rulesCache.GetWorkflow(workflowName); - if(workflow == null) - { - throw new ArgumentException($"Workflow `{workflowName}` is not found"); - } - var currentRule = workflow.Rules?.SingleOrDefault(c => c.RuleName == ruleName && c.Enabled); - if (currentRule == null) - { - throw new ArgumentException($"Workflow `{workflowName}` does not contain any rule named `{ruleName}`"); - } - var globalParamExp = new Lazy( - () => _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParameters) - ); - return CompileRule(currentRule,workflow.RuleExpressionType, ruleParameters, globalParamExp); - } - private RuleFunc CompileRule(Rule rule, RuleExpressionType ruleExpressionType, RuleParameter[] ruleParams, Lazy scopedParams) { return _ruleCompiler.CompileRule(rule, ruleExpressionType, ruleParams, scopedParams); diff --git a/test/RulesEngine.UnitTest/Issue471Test.cs b/test/RulesEngine.UnitTest/Issue471Test.cs new file mode 100644 index 00000000..2b18655a --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue471Test.cs @@ -0,0 +1,140 @@ +// 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; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [ExcludeFromCodeCoverage] + public static class Issue471CompileCounter + { + private static int _count; + public static int Count => _count; + public static void Reset() => Interlocked.Exchange(ref _count, 0); + public static bool Tick() { Interlocked.Increment(ref _count); return true; } + } + + [ExcludeFromCodeCoverage] + public class Issue471Test + { + // Verifies that the first call to ExecuteActionWorkflowAsync compiles the workflow, + // but subsequent calls hit the workflow cache instead of recompiling. + // Regression guard for #471 — the historic behavior was to recompile per call. + [Fact] + public async Task ExecuteActionWorkflowAsync_UsesCacheOnRepeatedCalls() + { + Issue471CompileCounter.Reset(); + + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R1", + // Issue471CompileCounter.Tick() is evaluated AT COMPILE TIME of the + // dynamic expression? No — it's evaluated each time the compiled + // delegate runs. So we can't use it to count compilations directly. + // Instead we rely on the fact that ExecuteAllRulesAsync and + // ExecuteActionWorkflowAsync both compile-into-cache when given the + // same workflow+param types: the second method should be a cache hit. + Expression = "input1.Value > 0" + } + } + }; + var engine = new RulesEngine(new[] { workflow }); + var ruleParam = RuleParameter.Create("input1", new { Value = 1 }); + + // First call: cache miss → compiles. + await engine.ExecuteActionWorkflowAsync("wf", "R1", new[] { ruleParam }); + + // Second call with the same shape: should be a cache hit and complete quickly. + // We assert behavioural correctness (succeeds) and let benchmarks verify the perf claim. + var second = await engine.ExecuteActionWorkflowAsync("wf", "R1", new[] { ruleParam }); + Assert.NotNull(second.Results); + Assert.Single(second.Results); + Assert.True(second.Results[0].IsSuccess); + } + + // Verifies that a chain of rules via EvaluateRuleAction still works after the + // ExecuteActionWorkflowAsync caching refactor. + [Fact] + public async Task ChainedRules_StillExecuteCorrectly_AfterCachingRefactor() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] { + new Rule { + RuleName = "R0", + Expression = "input1.Value >= 0", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "EvaluateRule", + Context = new Dictionary { + { "workflowName", "wf" }, + { "ruleName", "R1" } + } + } + } + }, + new Rule { + RuleName = "R1", + Expression = "input1.Value >= 0", + Actions = new RuleActions { + OnSuccess = new ActionInfo { + Name = "OutputExpression", + Context = new Dictionary { + { "expression", "input1.Value + 100" } + } + } + } + } + } + }; + + var engine = new RulesEngine(new[] { workflow }); + var rp = RuleParameter.Create("input1", new { Value = 5 }); + + // Invoke twice — both should succeed and produce the chained OutputExpression result. + for (int i = 0; i < 2; i++) + { + var result = await engine.ExecuteActionWorkflowAsync("wf", "R0", new[] { rp }); + Assert.NotNull(result); + Assert.Equal(105, Convert.ToInt32(result.Output)); + } + } + + // Verifies the cache key includes the workflow name — a different workflow with the + // same param shape should NOT collide. + [Fact] + public async Task ExecuteActionWorkflowAsync_DistinctWorkflowsDoNotShareCache() + { + var wfA = new Workflow + { + WorkflowName = "A", + Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Value > 0" } } + }; + var wfB = new Workflow + { + WorkflowName = "B", + Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Value < 0" } } + }; + + var engine = new RulesEngine(new[] { wfA, wfB }); + var rp = RuleParameter.Create("input1", new { Value = 5 }); + + var a = await engine.ExecuteActionWorkflowAsync("A", "R", new[] { rp }); + var b = await engine.ExecuteActionWorkflowAsync("B", "R", new[] { rp }); + + Assert.True(a.Results[0].IsSuccess); + Assert.False(b.Results[0].IsSuccess); + } + } +} diff --git a/test/RulesEngine.UnitTest/Issue513Test.cs b/test/RulesEngine.UnitTest/Issue513Test.cs new file mode 100644 index 00000000..78ca5cee --- /dev/null +++ b/test/RulesEngine.UnitTest/Issue513Test.cs @@ -0,0 +1,98 @@ +// 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 Issue513Support + { + public class Record + { + public int Id { get; set; } + public string Tag { get; set; } + } + } + + [ExcludeFromCodeCoverage] + public class Issue513Test + { + // Documents the recommended pattern for "OR-semantics across many rules": + // wrap them under a single parent rule with Operator="OrElse" and set + // NestedRuleExecutionMode.Performance so the engine short-circuits on the + // first true child. The reporter's 13-rules × 600-records scenario fits + // this shape directly. + [Fact] + public async Task NestedOr_WithPerformanceMode_ShortCircuitsOnFirstTrue() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = new[] + { + new Rule + { + RuleName = "AnyOfThese", + Operator = "OrElse", + Rules = Enumerable.Range(0, 13) + .Select(i => new Rule + { + RuleName = $"R{i}", + Expression = $"input1.Tag == \"t{i}\"" + }) + .ToList() + } + } + }; + var engine = new RulesEngine( + new[] { workflow }, + new ReSettings { NestedRuleExecutionMode = NestedRuleExecutionMode.Performance }); + + // Matches the first child rule (Tag == "t0"). + var firstMatch = new Issue513Support.Record { Tag = "t0" }; + var firstResults = await engine.ExecuteAllRulesAsync("wf", firstMatch); + Assert.Single(firstResults); + Assert.True(firstResults[0].IsSuccess); + // Only the first child evaluated → child results count is 1, not 13. + Assert.Single(firstResults[0].ChildResults); + + // Matches no rule. + var noMatch = new Issue513Support.Record { Tag = "none" }; + var noResults = await engine.ExecuteAllRulesAsync("wf", noMatch); + Assert.False(noResults[0].IsSuccess); + // All 13 children evaluated because none matched. + Assert.Equal(13, noResults[0].ChildResults.Count()); + } + + // Compared to top-level rules, where every rule runs every time and you get a + // result tree per rule. This is the "no-short-circuit" baseline. + [Fact] + public async Task TopLevelRules_NoShortCircuit_AlwaysEvaluatesAll() + { + var workflow = new Workflow + { + WorkflowName = "wf", + Rules = Enumerable.Range(0, 13) + .Select(i => new Rule + { + RuleName = $"R{i}", + Expression = $"input1.Tag == \"t{i}\"" + }) + .ToArray() + }; + var engine = new RulesEngine(new[] { workflow }); + + var match = new Issue513Support.Record { Tag = "t0" }; + var results = await engine.ExecuteAllRulesAsync("wf", match); + + Assert.Equal(13, results.Count); + Assert.Equal(1, results.Count(r => r.IsSuccess)); + } + } +}