Skip to content

Commit 3c5350d

Browse files
YogeshPrajYogesh Prajapati
andauthored
Fix #576, #519, #696, and add regression guards for #590, #608, #581, #606 (#729)
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. Co-authored-by: Yogesh Prajapati <yogeshcprajapati@outlook.com>
1 parent 29913a6 commit 3c5350d

9 files changed

Lines changed: 502 additions & 33 deletions

File tree

src/RulesEngine/Actions/ActionContext.cs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,31 @@ public class ActionContext
1717
public ActionContext(IDictionary<string, object> context, RuleResultTree parentResult)
1818
{
1919
_context = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
20-
foreach (var kv in context)
20+
if (context != null)
2121
{
22-
string key = kv.Key;
23-
string value;
24-
switch (kv.Value.GetType().Name)
22+
foreach (var kv in context)
2523
{
26-
case "String":
27-
case "JsonElement":
28-
value = kv.Value.ToString();
29-
break;
30-
default:
31-
value = JsonSerializer.Serialize(kv.Value);
32-
break;
33-
24+
string key = kv.Key;
25+
string value;
26+
if (kv.Value == null)
27+
{
28+
value = null;
29+
}
30+
else
31+
{
32+
switch (kv.Value.GetType().Name)
33+
{
34+
case "String":
35+
case "JsonElement":
36+
value = kv.Value.ToString();
37+
break;
38+
default:
39+
value = JsonSerializer.Serialize(kv.Value);
40+
break;
41+
}
42+
}
43+
_context.Add(key, value);
3444
}
35-
_context.Add(key, value);
3645
}
3746
_parentResult = parentResult;
3847
}

src/RulesEngine/RulesEngine.cs

Lines changed: 31 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ public async ValueTask<ActionRuleResult> ExecuteActionWorkflowAsync(string workf
132132
var compiledRule = CompileRule(workflowName, ruleName, ruleParameters);
133133
var extendedRuleParameters = EvaluateGlobalsAdHoc(workflowName, ruleParameters);
134134
var resultTree = compiledRule(extendedRuleParameters);
135+
// Mirror ExecuteAllRulesAsync's behavior: format the per-rule ErrorMessage template
136+
// into ExceptionMessage before any action runs / before returning. See #519.
137+
FormatErrorMessages(new[] { resultTree });
135138
var actionResult = await ExecuteActionForRuleResult(resultTree, true);
136139
ThrowIfActionExceptionShouldPropagate(actionResult, resultTree);
137140
return actionResult;
@@ -541,9 +544,7 @@ private IEnumerable<RuleResultTree> FormatErrorMessages(IEnumerable<RuleResultTr
541544
var property = paramVal?.Substring(2, paramVal.Length - 3);
542545
if (property?.Split('.')?.Count() > 1)
543546
{
544-
var typeName = property?.Split('.')?[0];
545-
var propertyName = property?.Split('.')?[1];
546-
errorMessage = UpdateErrorMessage(errorMessage, inputs, property, typeName, propertyName);
547+
errorMessage = UpdateErrorMessage(errorMessage, inputs, property);
547548
}
548549
else
549550
{
@@ -562,28 +563,38 @@ private IEnumerable<RuleResultTree> FormatErrorMessages(IEnumerable<RuleResultTr
562563
}
563564

564565
/// <summary>
565-
/// Updates the error message.
566+
/// Resolves a dotted-path placeholder like $(input1.Inner.Name) against the rule inputs,
567+
/// walking arbitrary depth. See #696.
566568
/// </summary>
567-
/// <param name="errorMessage">The error message.</param>
568-
/// <param name="evaluatedParams">The evaluated parameters.</param>
569-
/// <param name="property">The property.</param>
570-
/// <param name="typeName">Name of the type.</param>
571-
/// <param name="propertyName">Name of the property.</param>
572-
/// <returns>Updated error message.</returns>
573-
private static string UpdateErrorMessage(string errorMessage, IDictionary<string, object> inputs, string property, string typeName, string propertyName)
569+
private static string UpdateErrorMessage(string errorMessage, IDictionary<string, object> inputs, string property)
574570
{
575-
var arrParams = inputs?.Select(c => new { Name = c.Key, c.Value });
576-
var model = arrParams?.Where(a => string.Equals(a.Name, typeName))?.FirstOrDefault();
577-
if (model != null)
571+
var segments = property.Split('.');
572+
var typeName = segments[0];
573+
var model = inputs?.FirstOrDefault(c => string.Equals(c.Key, typeName));
574+
if (model?.Value == null)
575+
{
576+
return errorMessage;
577+
}
578+
579+
var modelJson = JsonSerializer.Serialize(model.Value.Value);
580+
JsonNode current = JsonNode.Parse(modelJson);
581+
for (var i = 1; i < segments.Length && current != null; i++)
582+
{
583+
current = current is JsonObject jObj && jObj.TryGetPropertyValue(segments[i], out var next)
584+
? next
585+
: null;
586+
}
587+
588+
if (current == null)
578589
{
579-
var modelJson = JsonSerializer.Serialize(model?.Value);
580-
var jObj = JsonObject.Parse(modelJson).AsObject();
581-
JsonNode jToken = null;
582-
var val = jObj?.TryGetPropertyValue(propertyName, out jToken);
583-
errorMessage = errorMessage.Replace($"$({property})", jToken != null ? jToken?.ToString() : $"({property})");
590+
return errorMessage;
584591
}
585592

586-
return errorMessage;
593+
// JsonValue (leaf scalar) should render without quotes; objects/arrays render as JSON.
594+
var replacement = current is JsonValue v && v.TryGetValue<string>(out var stringValue)
595+
? stringValue
596+
: current.ToString();
597+
return errorMessage.Replace($"$({property})", replacement);
587598
}
588599
#endregion
589600
}
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using RulesEngine.Models;
5+
using System.Diagnostics.CodeAnalysis;
6+
using System.Linq;
7+
using System.Threading.Tasks;
8+
using Xunit;
9+
10+
namespace RulesEngine.UnitTest
11+
{
12+
[ExcludeFromCodeCoverage]
13+
public class Issue519Test
14+
{
15+
[Fact]
16+
public async Task ExecuteActionWorkflowAsync_FailingRule_PopulatesExceptionMessageFromErrorMessage()
17+
{
18+
var workflow = new Workflow
19+
{
20+
WorkflowName = "wf",
21+
Rules = new[] {
22+
new Rule {
23+
RuleName = "R",
24+
Expression = "input1 == \"expected\"",
25+
ErrorMessage = "Input was $(input1), expected `expected`"
26+
}
27+
}
28+
};
29+
var engine = new RulesEngine(new[] { workflow });
30+
31+
var actionResult = await engine.ExecuteActionWorkflowAsync("wf", "R",
32+
new[] { RuleParameter.Create("input1", "actual-value") });
33+
34+
var ruleResult = actionResult.Results.Single();
35+
Assert.False(ruleResult.IsSuccess);
36+
// Before the fix, ExceptionMessage was empty; after, it should contain the interpolated ErrorMessage.
37+
Assert.False(string.IsNullOrEmpty(ruleResult.ExceptionMessage));
38+
Assert.Contains("actual-value", ruleResult.ExceptionMessage);
39+
}
40+
41+
[Fact]
42+
public async Task ExecuteAllRulesAsync_BehavesTheSameForReference()
43+
{
44+
var workflow = new Workflow
45+
{
46+
WorkflowName = "wf",
47+
Rules = new[] {
48+
new Rule {
49+
RuleName = "R",
50+
Expression = "input1 == \"expected\"",
51+
ErrorMessage = "Input was $(input1), expected `expected`"
52+
}
53+
}
54+
};
55+
var engine = new RulesEngine(new[] { workflow });
56+
var results = await engine.ExecuteAllRulesAsync("wf", "actual-value");
57+
58+
Assert.False(results[0].IsSuccess);
59+
Assert.Contains("actual-value", results[0].ExceptionMessage);
60+
}
61+
}
62+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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.Tasks;
10+
using Xunit;
11+
12+
namespace RulesEngine.UnitTest
13+
{
14+
[ExcludeFromCodeCoverage]
15+
public class Issue576Test
16+
{
17+
private class NoContextAction : ActionBase
18+
{
19+
public static bool WasRun;
20+
public override ValueTask<object> Run(ActionContext context, RuleParameter[] ruleParameters)
21+
{
22+
WasRun = true;
23+
return new ValueTask<object>("done");
24+
}
25+
}
26+
27+
[Fact]
28+
public async Task CustomAction_WithNullContext_DoesNotThrow()
29+
{
30+
NoContextAction.WasRun = false;
31+
32+
var workflow = new Workflow
33+
{
34+
WorkflowName = "wf",
35+
Rules = new[] {
36+
new Rule {
37+
RuleName = "R",
38+
Expression = "true",
39+
Actions = new RuleActions {
40+
OnSuccess = new ActionInfo {
41+
Name = "noctx",
42+
Context = null
43+
}
44+
}
45+
}
46+
}
47+
};
48+
var settings = new ReSettings
49+
{
50+
CustomActions = new Dictionary<string, Func<ActionBase>>
51+
{
52+
{ "noctx", () => new NoContextAction() }
53+
}
54+
};
55+
var engine = new RulesEngine(new[] { workflow }, settings);
56+
var results = await engine.ExecuteAllRulesAsync("wf", "x");
57+
58+
Assert.True(NoContextAction.WasRun, "Custom action should have run even with null Context");
59+
Assert.Null(results[0].ActionResult?.Exception);
60+
}
61+
62+
[Fact]
63+
public void ActionContext_NullDictionary_DoesNotThrow()
64+
{
65+
var ex = Record.Exception(() => new ActionContext(null, null));
66+
Assert.Null(ex);
67+
}
68+
}
69+
}
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using RulesEngine.Models;
5+
using System.Diagnostics.CodeAnalysis;
6+
using System.Threading.Tasks;
7+
using Xunit;
8+
9+
namespace RulesEngine.UnitTest
10+
{
11+
[ExcludeFromCodeCoverage]
12+
public class Issue581Support
13+
{
14+
public class MyParam
15+
{
16+
public string Value1 { get; set; }
17+
public string Value2 { get; set; }
18+
}
19+
}
20+
21+
[ExcludeFromCodeCoverage]
22+
public class Issue581Test
23+
{
24+
[Fact]
25+
public async Task CustomParameterName_IsHonored()
26+
{
27+
var workflow = new Workflow
28+
{
29+
WorkflowName = "my_workflow",
30+
Rules = new[] {
31+
new Rule {
32+
RuleName = "MatchesFabrikam",
33+
Enabled = true,
34+
RuleExpressionType = RuleExpressionType.LambdaExpression,
35+
Expression = "myValue.Value1 == \"Fabrikam\""
36+
}
37+
}
38+
};
39+
40+
var input = new Issue581Support.MyParam { Value1 = "Fabrikam", Value2 = "x" };
41+
var rp = new RuleParameter("myValue", input);
42+
var engine = new RulesEngine(new[] { workflow });
43+
44+
var results = await engine.ExecuteAllRulesAsync("my_workflow", new[] { rp });
45+
46+
Assert.True(results[0].IsSuccess,
47+
$"Expected success. Got: {results[0].ExceptionMessage}");
48+
}
49+
}
50+
}
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT License.
3+
4+
using RulesEngine.Models;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Diagnostics.CodeAnalysis;
8+
using System.Linq;
9+
using System.Threading.Tasks;
10+
using Xunit;
11+
12+
namespace RulesEngine.UnitTest
13+
{
14+
[ExcludeFromCodeCoverage]
15+
public class Issue590Test
16+
{
17+
public class FlakyInput
18+
{
19+
private int _counter;
20+
private string _simpleProp;
21+
public string SimpleProp
22+
{
23+
get
24+
{
25+
if (_counter++ == 0)
26+
{
27+
throw new ArgumentException("first-call-failure");
28+
}
29+
return _simpleProp;
30+
}
31+
set { _simpleProp = value; }
32+
}
33+
}
34+
35+
[Fact]
36+
public async Task ExceptionMessage_FromPriorRunDoesNotLeakIntoNextSuccessfulRun()
37+
{
38+
var input = new FlakyInput { SimpleProp = "simpleProp" };
39+
var workflow = new Workflow
40+
{
41+
WorkflowName = "wf",
42+
Rules = new[] {
43+
new Rule {
44+
RuleName = "CheckSimpleProp",
45+
RuleExpressionType = RuleExpressionType.LambdaExpression,
46+
Expression = "SimpleProp == \"simpleProp\"",
47+
ErrorMessage = "should not leak"
48+
}
49+
}
50+
};
51+
var settings = new ReSettings { UseFastExpressionCompiler = false };
52+
var engine = new RulesEngine(new[] { workflow }, settings);
53+
54+
var firstRun = await engine.ExecuteAllRulesAsync("wf", input);
55+
Assert.False(firstRun[0].IsSuccess);
56+
Assert.False(string.IsNullOrEmpty(firstRun[0].ExceptionMessage));
57+
58+
var secondRun = await engine.ExecuteAllRulesAsync("wf", input);
59+
Assert.True(secondRun[0].IsSuccess,
60+
$"Second run should succeed. Got ExceptionMessage = `{secondRun[0].ExceptionMessage}`");
61+
Assert.True(string.IsNullOrEmpty(secondRun[0].ExceptionMessage),
62+
$"Second run ExceptionMessage should be empty. Got `{secondRun[0].ExceptionMessage}`");
63+
}
64+
}
65+
}

0 commit comments

Comments
 (0)