asulwer / RulesEngine

Rules Engine with extensive Dynamic expression support
MIT License
26 stars 1 forks source link

Fix how empty arrays are handled by expressions #79

Closed JasonBoggsAtHMSdotCom closed 2 weeks ago

JasonBoggsAtHMSdotCom commented 2 weeks ago

Fixes https://github.com/asulwer/RulesEngine/issues/75

This PR fixes an issue with how empty arrays, provided via an expando object, are handled by the rules engine. System.Linq is checking the data type of the expected underlying object before comparing the contents of the actual object. This is causing issues when empty arrays are provided in json bodies, and these get converted to a List<object>. If you have a rule expression that looks something like widgets.Any(a == 3), and widgets is an empty array, the expression parser in System.Linq would experience an issue comparing an object to an integer.

To fix this issue, this PR creates a new class named ImplicitObject which contains implicit operators for every logical data type that can be passed in from a json object.

Run the following code:

using System.Collections;
using System.Dynamic;
using System.Text.Json;

using Newtonsoft.Json.Converters;

using RulesEngine.Models;

string payload = "{\"prop\":\"someString\",\"someInt\":3,\"nest\":{\"code\":\"bar\",\"foo\":true},\"emptyArray\":[],\"populatedArray\":[{\"a\":2,\"subArray\":[{\"c\":4}]}]}";

Workflow[] workflow = [
    new() {
        WorkflowName = "Workflow",
        Rules = [
            new() {
                RuleName = "someInt check",
                Expression = "someInt > 1",
                RuleExpressionType = RuleExpressionType.LambdaExpression
            },
            new() {
                RuleName = "empty array",
                Expression = "not emptyArray.Any(a == 'a')",
                RuleExpressionType = RuleExpressionType.LambdaExpression
            },
            new() {
                RuleName = "populatedArray with subArray not match",
                Expression = "populatedArray.Any(subArray.Any(c == 4))",
                RuleExpressionType = RuleExpressionType.LambdaExpression
            },
            new() {
                RuleName = "check prop",
                Expression = "prop = \"someString\"",
                RuleExpressionType = RuleExpressionType.LambdaExpression
            },
            new() {
                RuleName = "check nested code",
                Expression = "nest.code eq \"bar\" and nest.foo == true",
                RuleExpressionType = RuleExpressionType.LambdaExpression
            }
        ]
    }
];

var rulesEngine = new RulesEngine.RulesEngine(workflow, new() {
    IsExpressionCaseSensitive = false,
    CustomTypes = new[] {
        typeof(IEnumerable)
    }
});

var target = Newtonsoft.Json.JsonConvert.DeserializeObject<ExpandoObject>(payload, new ExpandoObjectConverter())!;

CancellationTokenSource cancellationTokenSource = new();
List<RuleResultTree> results = await rulesEngine.ExecuteAllRulesAsync("Workflow", cancellationTokenSource.Token, target);

Console.WriteLine(System.Text.Json.JsonSerializer.Serialize(target, new JsonSerializerOptions { WriteIndented = true }));
Console.WriteLine();

foreach(var result in results) {
    Console.WriteLine($"\t{result.IsSuccess}\t{result.Rule.Expression}");
    if(result.ExceptionMessage.Length > 0) Console.WriteLine($"\t\t\t{result.ExceptionMessage}");
}
asulwer commented 2 weeks ago

we have code coverage in place. basically, any new code needs a unit test. i think you must apply ExcludeFromCodeCoverage attribute to your class so it passes the code coverage in the build action check

asulwer commented 2 weeks ago

i am going to merge this but it will need the code coverage changes