perf mode for nested rules (#128)

* added perf mode for nested rules

* added more tests

* Added comments for NestedRuleExecutionMode and fixed typo

* updated readme

CHANGELOG.md

@@ -2,19 +2,14 @@
 
 All notable changes to this project will be documented in this file.
 
-## [3.1.0-preview.3]
-- Fixed scoped parameters runtime errors not logging as errorMessage
-
-## [3.1.0-preview.2]
-- Runtime errors for expressions will now be logged as errorMessage instead of throwing Exceptions by default
-
-## [3.1.0-preview.1]
+## [3.1.0]
 - Added globalParams feature which can be applied to all rules
 - Enabled localParams support for nested Rules
 - Made certain fields in Rule model optional allowing users to define workflow with minimal fields
 - Added option to disable Rule in workflow json
 - Added `GetAllRegisteredWorkflow` to RulesEngine to return all registeredWorkflows
-- Fixed Rule compilation exception not returned when Rule has ErrorMessage field defined - #95
+- Runtime errors for expressions will now be logged as errorMessage instead of throwing Exceptions by default
+- Fixed RuleParameter passed as null
 
 ## [3.0.2]
 - Fixed LocalParams cache not getting cleaned up when RemoveWorkflow and ClearWorkflows are called

src/RulesEngine/Models/ReSettings.cs

@@ -44,6 +44,11 @@ namespace RulesEngine.Models
         /// </summary>
         public bool EnableScopedParams { get; set; } = true;
 
+        /// <summary>
+        /// Sets the mode for Nested rule execution, Default: All
+        /// </summary>
+        public NestedRuleExecutionMode NestedRuleExecutionMode { get; set; } = NestedRuleExecutionMode.All;
+
         /// <summary>
         /// Enables Local params for rules
         /// </summary>
@@ -53,4 +58,16 @@ namespace RulesEngine.Models
             set { EnableScopedParams = value; }
         }
     }
+
+    public enum NestedRuleExecutionMode
+    {
+        /// <summary>
+        /// Excutes all nested rules
+        /// </summary>
+        All,
+        /// <summary>
+        /// Skips nested rules whose execution does not impact parent rule's result
+        /// </summary>
+        Performance
+    }
 }

src/RulesEngine/Models/Rule.cs

@@ -41,8 +41,8 @@ namespace RulesEngine.Models
 
         [JsonConverter(typeof(StringEnumConverter))]
         public RuleExpressionType RuleExpressionType { get; set; } = RuleExpressionType.LambdaExpression;
-        public List<string> WorkflowRulesToInject { get; set; }
-        public List<Rule> Rules { get; set; }
+        public IEnumerable<string> WorkflowRulesToInject { get; set; }
+        public IEnumerable<Rule> Rules { get; set; }
         public IEnumerable<ScopedParam> LocalParams { get; set; }
         public string Expression { get; set; }
         public Dictionary<ActionTriggerType, ActionInfo> Actions { get; set; }

src/RulesEngine/RuleCompiler.cs

@@ -186,33 +186,56 @@ namespace RulesEngine
             }
 
             return (paramArray) => {
-                var resultList = ruleFuncList.Select(fn => fn(paramArray)).ToList();
-                Func<object[], bool> isSuccess = (p) => ApplyOperation(resultList, operation);
-                var result = Helpers.ToResultTree(_reSettings, parentRule, resultList, isSuccess);
+                var (isSuccess, resultList) = ApplyOperation(paramArray, ruleFuncList, operation);
+                Func<object[], bool> isSuccessFn = (p) => isSuccess;
+                var result = Helpers.ToResultTree(_reSettings, parentRule, resultList, isSuccessFn);
                 return result(paramArray);
             };
         }
 
 
-        private bool ApplyOperation(IEnumerable<RuleResultTree> ruleResults, ExpressionType operation)
+        private (bool isSuccess ,IEnumerable<RuleResultTree> result) ApplyOperation(RuleParameter[] paramArray,IEnumerable<RuleFunc<RuleResultTree>> ruleFuncList, ExpressionType operation)
         {
-            if (ruleResults?.Any() != true)
+            if (ruleFuncList?.Any() != true)
             {
-                return false;
+                return (false,new List<RuleResultTree>());
             }
 
-            switch (operation)
-            {
-                case ExpressionType.And:
-                case ExpressionType.AndAlso:
-                    return ruleResults.All(r => r.IsSuccess);
+            var resultList = new List<RuleResultTree>();
+            var isSuccess = false;
 
-                case ExpressionType.Or:
-                case ExpressionType.OrElse:
-                    return ruleResults.Any(r => r.IsSuccess);
-                default:
-                    return false;
+            if(operation == ExpressionType.And || operation == ExpressionType.AndAlso)
+            {
+                isSuccess = true;
             }
+
+            foreach(var ruleFunc in ruleFuncList)
+            {
+                var ruleResult = ruleFunc(paramArray);
+                resultList.Add(ruleResult);
+                switch (operation)
+                {
+                    case ExpressionType.And:
+                    case ExpressionType.AndAlso:
+                        isSuccess = isSuccess && ruleResult.IsSuccess;
+                        if(_reSettings.NestedRuleExecutionMode ==  NestedRuleExecutionMode.Performance && isSuccess == false)
+                        {
+                            return (isSuccess, resultList);
+                        }
+                        break;
+
+                    case ExpressionType.Or:
+                    case ExpressionType.OrElse:
+                        isSuccess = isSuccess || ruleResult.IsSuccess;
+                        if (_reSettings.NestedRuleExecutionMode == NestedRuleExecutionMode.Performance && isSuccess == true)
+                        {
+                            return (isSuccess, resultList);
+                        }
+                        break;
+                }
+                
+            }
+            return (isSuccess, resultList);
         }
 
         private RuleFunc<RuleResultTree> GetWrappedRuleFunc(Rule rule, RuleFunc<RuleResultTree> ruleFunc,RuleParameter[] ruleParameters,RuleExpressionParameter[] ruleExpParams)
@@ -232,8 +255,9 @@ namespace RulesEngine
                     scopedParams = scopedParamsDict.Select(c => new RuleParameter(c.Key, c.Value));
                 }
                 catch(Exception ex)
-                { 
-                    var resultFn = Helpers.ToResultTree(_reSettings, rule, null, (args) => false, $"Error while executing scoped params for rule `{rule.RuleName}` - {ex}");
+                {
+                    var message = $"Error while executing scoped params for rule `{rule.RuleName}` - {ex}";
+                    var resultFn = Helpers.ToRuleExceptionResult(_reSettings, rule, new RuleException(message, ex));
                     return resultFn(ruleParams);
                 }
                

src/RulesEngine/Validators/RuleValidator.cs

@@ -43,7 +43,7 @@ namespace RulesEngine.Validators
             });
         }
 
-        private bool BeValidRulesList(List<Rule> rules)
+        private bool BeValidRulesList(IEnumerable<Rule> rules)
         {
             if (rules?.Any() != true) return false;
             var validator = new RuleValidator();

test/RulesEngine.UnitTest/LambdaExpressionBuilderTest.cs

@@ -5,6 +5,7 @@ using RulesEngine.ExpressionBuilders;
 using RulesEngine.Models;
 using System.Collections.Generic;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using Xunit;
 
 namespace RulesEngine.UnitTest
@@ -39,7 +40,7 @@ namespace RulesEngine.UnitTest
                 Expression = "RequestType == \"vod\""
             };
 
-            mainRule.Rules.Add(dummyRule);
+            mainRule.Rules = mainRule.Rules.Append(dummyRule);
             var func = builder.BuildDelegateForRule(dummyRule, ruleParameters);
 
             Assert.NotNull(func);

test/RulesEngine.UnitTest/NestedRulesTest.cs

@@ -0,0 +1,141 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using RulesEngine.Models;
+using System.Collections.Generic;
+using System.Diagnostics.CodeAnalysis;
+using System.Dynamic;
+using System.Linq;
+using System.Threading.Tasks;
+using Xunit;
+
+namespace RulesEngine.UnitTest
+{
+    [ExcludeFromCodeCoverage]
+    public class NestedRulesTest
+    {
+
+        [Theory]
+        [InlineData(NestedRuleExecutionMode.All)]
+        [InlineData(NestedRuleExecutionMode.Performance)]
+        public async Task NestedRulesShouldFollowExecutionMode(NestedRuleExecutionMode mode)
+        {
+            var workflows = GetWorkflows();
+            var reSettings = new ReSettings { NestedRuleExecutionMode = mode};
+            var rulesEngine = new RulesEngine(workflows, reSettings:reSettings);
+            dynamic input1 = new ExpandoObject();
+            input1.trueValue = true;
+
+            List<RuleResultTree> result = await rulesEngine.ExecuteAllRulesAsync("NestedRulesTest", input1);
+            var andResults = result.Where(c => c.Rule.Operator == "And").ToList();
+            var orResults = result.Where(c => c.Rule.Operator == "Or").ToList();
+            Assert.All(andResults,
+                c => Assert.False(c.IsSuccess)
+                );
+            Assert.All(orResults, 
+                c => Assert.True(c.IsSuccess));
+
+            if(mode == NestedRuleExecutionMode.All)
+            {
+                Assert.All(andResults,
+                    c => Assert.Equal(c.Rule.Rules.Count(), c.ChildResults.Count()));
+                Assert.All(orResults,
+                    c => Assert.Equal(c.Rule.Rules.Count(), c.ChildResults.Count()));
+            }
+            else if (mode == NestedRuleExecutionMode.Performance)
+            {
+                Assert.All(andResults,
+                    c => {
+                        Assert.Equal(c.IsSuccess, c.ChildResults.Last().IsSuccess);
+                        Assert.Single(c.ChildResults.Where(d => c.IsSuccess == d.IsSuccess));
+                        Assert.True(c.ChildResults.SkipLast(1).All(d => d.IsSuccess == true));
+                    });
+
+                Assert.All(orResults,
+                    c => {
+                        Assert.Equal(c.IsSuccess, c.ChildResults.Last().IsSuccess);
+                        Assert.Single(c.ChildResults.Where(d => c.IsSuccess == d.IsSuccess));
+                        Assert.True(c.ChildResults.SkipLast(1).All(d => d.IsSuccess == false));
+                    });
+
+            }
+
+            
+        }
+
+
+
+
+        private WorkflowRules[] GetWorkflows()
+        {
+            return new[] {
+                new WorkflowRules {
+                    WorkflowName = "NestedRulesTest",
+                    Rules = new Rule[] {
+                        new Rule {
+                            RuleName = "AndRuleTrueFalse",
+                            Operator = "And",
+                            Rules = new Rule[] {
+                                new Rule{
+                                    RuleName = "trueRule1",
+                                    Expression = "input1.TrueValue == true",
+                                },
+                                new Rule {
+                                    RuleName = "falseRule1",
+                                    Expression = "input1.TrueValue == false"
+                                }
+
+                            }
+                        },
+                        new Rule {
+                            RuleName = "OrRuleTrueFalse",
+                            Operator = "Or",
+                            Rules = new Rule[] {
+                                new Rule{
+                                    RuleName = "trueRule2",
+                                    Expression = "input1.TrueValue == true",
+                                },
+                                new Rule {
+                                    RuleName = "falseRule2",
+                                    Expression = "input1.TrueValue == false"
+                                }
+
+                            }
+                        },
+                        new Rule {
+                            RuleName = "AndRuleFalseTrue",
+                            Operator = "And",
+                            Rules = new Rule[] {
+                                new Rule{
+                                    RuleName = "trueRule3",
+                                    Expression = "input1.TrueValue == false",
+                                },
+                                new Rule {
+                                    RuleName = "falseRule4",
+                                    Expression = "input1.TrueValue == true"
+                                }
+
+                            }
+                        },
+                         new Rule {
+                            RuleName = "OrRuleFalseTrue",
+                            Operator = "Or",
+                            Rules = new Rule[] {
+                                new Rule{
+                                    RuleName = "trueRule3",
+                                    Expression = "input1.TrueValue == false",
+                                },
+                                new Rule {
+                                    RuleName = "falseRule4",
+                                    Expression = "input1.TrueValue == true"
+                                }
+
+                            }
+                         }
+                    }
+                },
+                
+            };
+        }
+    }
+}

test/RulesEngine.UnitTest/ScopedParamsTest.cs

@@ -70,14 +70,14 @@ namespace RulesEngine.UnitTest
 
 
         [Theory]
-        [InlineData("GlobalParamsOnly",new []{ false })]
+        [InlineData("GlobalParamsOnly", new[] { false })]
         [InlineData("LocalParamsOnly", new[] { false, true })]
         [InlineData("GlobalAndLocalParams", new[] { false })]
         public async Task DisabledScopedParam_ShouldReflect(string workflowName, bool[] outputs)
         {
             var workflows = GetWorkflowRulesList();
 
-            var engine = new RulesEngine(new string[] { }, null, new ReSettings { 
+            var engine = new RulesEngine(new string[] { }, null, new ReSettings {
                 EnableScopedParams = false
             });
             engine.AddWorkflow(workflows);
@@ -88,10 +88,10 @@ namespace RulesEngine.UnitTest
             };
 
             var result = await engine.ExecuteAllRulesAsync(workflowName, input1);
-            for(var i = 0; i < result.Count; i++)
+            for (var i = 0; i < result.Count; i++)
             {
                 Assert.Equal(result[i].IsSuccess, outputs[i]);
-                if(result[i].IsSuccess == false)
+                if (result[i].IsSuccess == false)
                 {
                     Assert.StartsWith("Exception while parsing expression", result[i].ExceptionMessage);
                 }
@@ -100,7 +100,7 @@ namespace RulesEngine.UnitTest
 
         [Theory]
         [InlineData("GlobalParamsOnly")]
-        [InlineData("LocalParamsOnly")]
+        [InlineData("LocalParamsOnly2")]
         public async Task ErrorInScopedParam_ShouldAppearAsErrorMessage(string workflowName)
         {
             var workflows = GetWorkflowRulesList();
@@ -111,7 +111,33 @@ namespace RulesEngine.UnitTest
             var input = new { };
             var result = await engine.ExecuteAllRulesAsync(workflowName, input);
 
-            Assert.All(result, c => Assert.False(c.IsSuccess));
+            Assert.All(result, c => { 
+                Assert.False(c.IsSuccess);
+                Assert.StartsWith("Error while compiling rule", c.ExceptionMessage);
+            });
+
+        }
+
+        [Theory]
+        [InlineData("GlobalParamsOnlyWithComplexInput")]
+        [InlineData("LocalParamsOnlyWithComplexInput")]
+        public async Task RuntimeErrorInScopedParam_ShouldAppearAsErrorMessage(string workflowName)
+        {
+            var workflows = GetWorkflowRulesList();
+
+            var engine = new RulesEngine(new string[] { }, null);
+            engine.AddWorkflow(workflows);
+
+
+
+            var input = new RuleTestClass();
+            var result = await engine.ExecuteAllRulesAsync(workflowName, input);
+
+            Assert.All(result, c => {
+                Assert.False(c.IsSuccess);
+                Assert.StartsWith("Error while executing scoped params for rule", c.ExceptionMessage);
+            });
+
 
         }
 
@@ -183,6 +209,23 @@ namespace RulesEngine.UnitTest
                         },
                     }
                 },
+                new WorkflowRules {
+                    WorkflowName = "LocalParamsOnly2",
+                    Rules = new List<Rule> {
+                        new Rule {
+
+                            RuleName = "WithLocalParam",
+                            LocalParams = new List<ScopedParam> {
+                                new ScopedParam {
+                                    Name = "localParam1",
+                                    Expression = "input1.trueValue"
+                                }
+                            },
+                            Expression = "localParam1 == true"
+                        }
+                    }
+                },
+
                 new WorkflowRules {
                     WorkflowName = "GlobalParamsOnly",
                     GlobalParams = new List<ScopedParam> {
@@ -296,7 +339,7 @@ namespace RulesEngine.UnitTest
                                 new ScopedParam {
                                     Name = "localParam1",
                                     Expression = @"""world"""
-                                } 
+                                }
                            },
                            Rules =  new List<Rule>{
                                new Rule{
@@ -318,7 +361,38 @@ namespace RulesEngine.UnitTest
 
                         }
                     }
-                }
+                },
+                new WorkflowRules {
+                    WorkflowName = "LocalParamsOnlyWithComplexInput",
+                    Rules = new List<Rule> {
+                        new Rule {
+
+                            RuleName = "WithLocalParam",
+                            LocalParams = new List<ScopedParam> {
+                                new ScopedParam {
+                                    Name = "localParam1",
+                                    Expression = "input1.Country.ToLower()"
+                                }
+                            },
+                            Expression = "localParam1 == \"hello\""
+                        }
+                    }
+                },
+                new WorkflowRules {
+                    WorkflowName = "GlobalParamsOnlyWithComplexInput",
+                    GlobalParams = new List<ScopedParam> {
+                        new ScopedParam {
+                            Name = "globalParam1",
+                            Expression = "input1.Country.ToLower()"
+                        }
+                    },
+                    Rules = new List<Rule> {
+                        new Rule {
+                            RuleName = "TrueTest",
+                            Expression = "globalParam1 == \"hello\""
+                        }
+                    }
+                },
             };
         }
     }