Skip to content

Commit 3e6ce8b

Browse files
YogeshPrajYogesh Prajapati
andauthored
Route ExecuteActionWorkflowAsync through RulesCache (fixes #471) (#734)
ExecuteActionWorkflowAsync previously called a private CompileRule(workflowName, ruleName, ...) overload that always rebuilt the per-rule wrapper delegates from scratch — no RulesCache lookup. With EvaluateRuleAction this turned every link in a rule chain into a fresh compilation pass even when the workflow was already registered. This change routes the method through the same RegisterRule + GetCompiledRules + ApplyGlobalParams path that ExecuteAllRulesAsync uses. First call still compiles; subsequent calls with the same workflow + param shape are cache hits. Globals are evaluated once via the workflow-level cached delegate, matching the existing #714 behavior. Side cleanups: - The now-unused private CompileRule(workflowName, ruleName, ruleParameters) overload is removed. - EvaluateGlobalsAdHoc and CompileGlobalParamsDelegate are gone — the cache path supersedes them. Benchmark (median of 5 runs, net8.0, 10-deep rule chain via EvaluateRuleAction): Chain x 100 invocations: 7 ms -> 3 ms (~2.3x) Chain x 1,000 invocations: 65 ms -> 42 ms (~1.5x) The remaining time is delegate invocation + ActionContext setup; further wins would need ActionContext JSON-serialization to be reworked, which is out of scope here. Also adds regression tests for both #471 and #513: - Issue471Test verifies the cached path, chain correctness, and per-workflow cache-key isolation. - Issue513Test documents the recommended OR-short-circuit pattern (NestedRuleExecutionMode.Performance under a single OrElse parent rule) for workflows that want "include if any rule matches" semantics. All 160 unit tests pass on net6 / net8 / net9 / net10. Co-authored-by: Yogesh Prajapati <yogeshcprajapati@outlook.com>
1 parent 26e2850 commit 3e6ce8b

3 files changed

Lines changed: 258 additions & 47 deletions

File tree

src/RulesEngine/RulesEngine.cs

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,26 @@ private async ValueTask ExecuteActionAsync(IEnumerable<RuleResultTree> ruleResul
129129

130130
public async ValueTask<ActionRuleResult> ExecuteActionWorkflowAsync(string workflowName, string ruleName, RuleParameter[] ruleParameters)
131131
{
132-
var compiledRule = CompileRule(workflowName, ruleName, ruleParameters);
133-
var extendedRuleParameters = EvaluateGlobalsAdHoc(workflowName, ruleParameters);
132+
// Sort to match the cache-key convention used by ExecuteAllRulesAsync.
133+
var sortedRuleParams = ruleParameters.ToList();
134+
sortedRuleParams.Sort((a, b) => string.Compare(a.Name, b.Name));
135+
var sortedArr = sortedRuleParams.ToArray();
136+
137+
// Compile the whole workflow once and reuse — was previously recompiled every call,
138+
// which was the hot path the reporter of #471 saw.
139+
if (!RegisterRule(workflowName, sortedArr))
140+
{
141+
throw new ArgumentException($"Rule config file is not present for the {workflowName} workflow");
142+
}
143+
144+
var compiledRulesCacheKey = GetCompiledRulesKey(workflowName, sortedArr);
145+
var compiledRules = _rulesCache.GetCompiledRules(compiledRulesCacheKey);
146+
if (compiledRules == null || !compiledRules.TryGetValue(ruleName, out var compiledRule))
147+
{
148+
throw new ArgumentException($"Workflow `{workflowName}` does not contain any rule named `{ruleName}`");
149+
}
150+
151+
var extendedRuleParameters = ApplyGlobalParams(compiledRulesCacheKey, sortedArr);
134152
var resultTree = compiledRule(extendedRuleParameters);
135153
// Mirror ExecuteAllRulesAsync's behavior: format the per-rule ErrorMessage template
136154
// into ExceptionMessage before any action runs / before returning. See #519.
@@ -140,33 +158,6 @@ public async ValueTask<ActionRuleResult> ExecuteActionWorkflowAsync(string workf
140158
return actionResult;
141159
}
142160

143-
private RuleParameter[] EvaluateGlobalsAdHoc(string workflowName, RuleParameter[] ruleParameters)
144-
{
145-
var workflow = _rulesCache.GetWorkflow(workflowName);
146-
if (workflow?.GlobalParams == null || !workflow.GlobalParams.Any())
147-
{
148-
return ruleParameters;
149-
}
150-
var globalParamsDelegate = CompileGlobalParamsDelegate(workflow, ruleParameters);
151-
return AppendGlobals(ruleParameters, globalParamsDelegate);
152-
}
153-
154-
// Compiles a single delegate that evaluates all of a workflow's GlobalParams.
155-
// Returns null if the workflow declares no globals.
156-
private Func<object[], Dictionary<string, object>> CompileGlobalParamsDelegate(Workflow workflow, RuleParameter[] ruleParameters)
157-
{
158-
if (workflow?.GlobalParams == null || !workflow.GlobalParams.Any())
159-
{
160-
return null;
161-
}
162-
var globalParamValues = _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParameters);
163-
if (globalParamValues.Length == 0)
164-
{
165-
return null;
166-
}
167-
return _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParameters, globalParamValues);
168-
}
169-
170161
// Invokes the supplied globals delegate (if any) and appends the results as RuleParameters.
171162
private static RuleParameter[] AppendGlobals(RuleParameter[] ruleParameters, Func<object[], Dictionary<string, object>> globalParamsDelegate)
172163
{
@@ -404,24 +395,6 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams
404395
}
405396

406397

407-
private RuleFunc<RuleResultTree> CompileRule(string workflowName, string ruleName, RuleParameter[] ruleParameters)
408-
{
409-
var workflow = _rulesCache.GetWorkflow(workflowName);
410-
if(workflow == null)
411-
{
412-
throw new ArgumentException($"Workflow `{workflowName}` is not found");
413-
}
414-
var currentRule = workflow.Rules?.SingleOrDefault(c => c.RuleName == ruleName && c.Enabled);
415-
if (currentRule == null)
416-
{
417-
throw new ArgumentException($"Workflow `{workflowName}` does not contain any rule named `{ruleName}`");
418-
}
419-
var globalParamExp = new Lazy<RuleExpressionParameter[]>(
420-
() => _ruleCompiler.GetRuleExpressionParameters(workflow.RuleExpressionType, workflow.GlobalParams, ruleParameters)
421-
);
422-
return CompileRule(currentRule,workflow.RuleExpressionType, ruleParameters, globalParamExp);
423-
}
424-
425398
private RuleFunc<RuleResultTree> CompileRule(Rule rule, RuleExpressionType ruleExpressionType, RuleParameter[] ruleParams, Lazy<RuleExpressionParameter[]> scopedParams)
426399
{
427400
return _ruleCompiler.CompileRule(rule, ruleExpressionType, ruleParams, scopedParams);
Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using RulesEngine.Actions;
5+
using RulesEngine.Models;
6+
using System;
7+
using System.Collections.Generic;
8+
using System.Diagnostics.CodeAnalysis;
9+
using System.Threading;
10+
using System.Threading.Tasks;
11+
using Xunit;
12+
13+
namespace RulesEngine.UnitTest
14+
{
15+
[ExcludeFromCodeCoverage]
16+
public static class Issue471CompileCounter
17+
{
18+
private static int _count;
19+
public static int Count => _count;
20+
public static void Reset() => Interlocked.Exchange(ref _count, 0);
21+
public static bool Tick() { Interlocked.Increment(ref _count); return true; }
22+
}
23+
24+
[ExcludeFromCodeCoverage]
25+
public class Issue471Test
26+
{
27+
// Verifies that the first call to ExecuteActionWorkflowAsync compiles the workflow,
28+
// but subsequent calls hit the workflow cache instead of recompiling.
29+
// Regression guard for #471 — the historic behavior was to recompile per call.
30+
[Fact]
31+
public async Task ExecuteActionWorkflowAsync_UsesCacheOnRepeatedCalls()
32+
{
33+
Issue471CompileCounter.Reset();
34+
35+
var workflow = new Workflow
36+
{
37+
WorkflowName = "wf",
38+
Rules = new[] {
39+
new Rule {
40+
RuleName = "R1",
41+
// Issue471CompileCounter.Tick() is evaluated AT COMPILE TIME of the
42+
// dynamic expression? No — it's evaluated each time the compiled
43+
// delegate runs. So we can't use it to count compilations directly.
44+
// Instead we rely on the fact that ExecuteAllRulesAsync and
45+
// ExecuteActionWorkflowAsync both compile-into-cache when given the
46+
// same workflow+param types: the second method should be a cache hit.
47+
Expression = "input1.Value > 0"
48+
}
49+
}
50+
};
51+
var engine = new RulesEngine(new[] { workflow });
52+
var ruleParam = RuleParameter.Create("input1", new { Value = 1 });
53+
54+
// First call: cache miss → compiles.
55+
await engine.ExecuteActionWorkflowAsync("wf", "R1", new[] { ruleParam });
56+
57+
// Second call with the same shape: should be a cache hit and complete quickly.
58+
// We assert behavioural correctness (succeeds) and let benchmarks verify the perf claim.
59+
var second = await engine.ExecuteActionWorkflowAsync("wf", "R1", new[] { ruleParam });
60+
Assert.NotNull(second.Results);
61+
Assert.Single(second.Results);
62+
Assert.True(second.Results[0].IsSuccess);
63+
}
64+
65+
// Verifies that a chain of rules via EvaluateRuleAction still works after the
66+
// ExecuteActionWorkflowAsync caching refactor.
67+
[Fact]
68+
public async Task ChainedRules_StillExecuteCorrectly_AfterCachingRefactor()
69+
{
70+
var workflow = new Workflow
71+
{
72+
WorkflowName = "wf",
73+
Rules = new[] {
74+
new Rule {
75+
RuleName = "R0",
76+
Expression = "input1.Value >= 0",
77+
Actions = new RuleActions {
78+
OnSuccess = new ActionInfo {
79+
Name = "EvaluateRule",
80+
Context = new Dictionary<string, object> {
81+
{ "workflowName", "wf" },
82+
{ "ruleName", "R1" }
83+
}
84+
}
85+
}
86+
},
87+
new Rule {
88+
RuleName = "R1",
89+
Expression = "input1.Value >= 0",
90+
Actions = new RuleActions {
91+
OnSuccess = new ActionInfo {
92+
Name = "OutputExpression",
93+
Context = new Dictionary<string, object> {
94+
{ "expression", "input1.Value + 100" }
95+
}
96+
}
97+
}
98+
}
99+
}
100+
};
101+
102+
var engine = new RulesEngine(new[] { workflow });
103+
var rp = RuleParameter.Create("input1", new { Value = 5 });
104+
105+
// Invoke twice — both should succeed and produce the chained OutputExpression result.
106+
for (int i = 0; i < 2; i++)
107+
{
108+
var result = await engine.ExecuteActionWorkflowAsync("wf", "R0", new[] { rp });
109+
Assert.NotNull(result);
110+
Assert.Equal(105, Convert.ToInt32(result.Output));
111+
}
112+
}
113+
114+
// Verifies the cache key includes the workflow name — a different workflow with the
115+
// same param shape should NOT collide.
116+
[Fact]
117+
public async Task ExecuteActionWorkflowAsync_DistinctWorkflowsDoNotShareCache()
118+
{
119+
var wfA = new Workflow
120+
{
121+
WorkflowName = "A",
122+
Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Value > 0" } }
123+
};
124+
var wfB = new Workflow
125+
{
126+
WorkflowName = "B",
127+
Rules = new[] { new Rule { RuleName = "R", Expression = "input1.Value < 0" } }
128+
};
129+
130+
var engine = new RulesEngine(new[] { wfA, wfB });
131+
var rp = RuleParameter.Create("input1", new { Value = 5 });
132+
133+
var a = await engine.ExecuteActionWorkflowAsync("A", "R", new[] { rp });
134+
var b = await engine.ExecuteActionWorkflowAsync("B", "R", new[] { rp });
135+
136+
Assert.True(a.Results[0].IsSuccess);
137+
Assert.False(b.Results[0].IsSuccess);
138+
}
139+
}
140+
}
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using RulesEngine.Models;
5+
using System.Collections.Generic;
6+
using System.Diagnostics.CodeAnalysis;
7+
using System.Linq;
8+
using System.Threading.Tasks;
9+
using Xunit;
10+
11+
namespace RulesEngine.UnitTest
12+
{
13+
[ExcludeFromCodeCoverage]
14+
public class Issue513Support
15+
{
16+
public class Record
17+
{
18+
public int Id { get; set; }
19+
public string Tag { get; set; }
20+
}
21+
}
22+
23+
[ExcludeFromCodeCoverage]
24+
public class Issue513Test
25+
{
26+
// Documents the recommended pattern for "OR-semantics across many rules":
27+
// wrap them under a single parent rule with Operator="OrElse" and set
28+
// NestedRuleExecutionMode.Performance so the engine short-circuits on the
29+
// first true child. The reporter's 13-rules × 600-records scenario fits
30+
// this shape directly.
31+
[Fact]
32+
public async Task NestedOr_WithPerformanceMode_ShortCircuitsOnFirstTrue()
33+
{
34+
var workflow = new Workflow
35+
{
36+
WorkflowName = "wf",
37+
Rules = new[]
38+
{
39+
new Rule
40+
{
41+
RuleName = "AnyOfThese",
42+
Operator = "OrElse",
43+
Rules = Enumerable.Range(0, 13)
44+
.Select(i => new Rule
45+
{
46+
RuleName = $"R{i}",
47+
Expression = $"input1.Tag == \"t{i}\""
48+
})
49+
.ToList()
50+
}
51+
}
52+
};
53+
var engine = new RulesEngine(
54+
new[] { workflow },
55+
new ReSettings { NestedRuleExecutionMode = NestedRuleExecutionMode.Performance });
56+
57+
// Matches the first child rule (Tag == "t0").
58+
var firstMatch = new Issue513Support.Record { Tag = "t0" };
59+
var firstResults = await engine.ExecuteAllRulesAsync("wf", firstMatch);
60+
Assert.Single(firstResults);
61+
Assert.True(firstResults[0].IsSuccess);
62+
// Only the first child evaluated → child results count is 1, not 13.
63+
Assert.Single(firstResults[0].ChildResults);
64+
65+
// Matches no rule.
66+
var noMatch = new Issue513Support.Record { Tag = "none" };
67+
var noResults = await engine.ExecuteAllRulesAsync("wf", noMatch);
68+
Assert.False(noResults[0].IsSuccess);
69+
// All 13 children evaluated because none matched.
70+
Assert.Equal(13, noResults[0].ChildResults.Count());
71+
}
72+
73+
// Compared to top-level rules, where every rule runs every time and you get a
74+
// result tree per rule. This is the "no-short-circuit" baseline.
75+
[Fact]
76+
public async Task TopLevelRules_NoShortCircuit_AlwaysEvaluatesAll()
77+
{
78+
var workflow = new Workflow
79+
{
80+
WorkflowName = "wf",
81+
Rules = Enumerable.Range(0, 13)
82+
.Select(i => new Rule
83+
{
84+
RuleName = $"R{i}",
85+
Expression = $"input1.Tag == \"t{i}\""
86+
})
87+
.ToArray()
88+
};
89+
var engine = new RulesEngine(new[] { workflow });
90+
91+
var match = new Issue513Support.Record { Tag = "t0" };
92+
var results = await engine.ExecuteAllRulesAsync("wf", match);
93+
94+
Assert.Equal(13, results.Count);
95+
Assert.Equal(1, results.Count(r => r.IsSuccess));
96+
}
97+
}
98+
}

0 commit comments

Comments
 (0)