Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix how empty arrays are handled by expressions #79

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

JasonBoggsAtHMSdotCom
Copy link
Collaborator

@JasonBoggsAtHMSdotCom JasonBoggsAtHMSdotCom commented Nov 13, 2024

Fixes #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
Copy link
Owner

asulwer commented Nov 13, 2024

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
Copy link
Owner

asulwer commented Nov 13, 2024

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

@asulwer asulwer merged commit 1f09727 into asulwer:main Nov 13, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Struggling with empty arrays
2 participants