From 331776d3e7029c39478c07389aed68c4842fae57 Mon Sep 17 00:00:00 2001 From: Abbas Cyclewala Date: Tue, 27 Apr 2021 12:17:42 +0530 Subject: [PATCH] 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 | 11 +- src/RulesEngine/Models/ReSettings.cs | 17 +++ src/RulesEngine/Models/Rule.cs | 4 +- src/RulesEngine/RuleCompiler.cs | 60 ++++++--- src/RulesEngine/Validators/RuleValidator.cs | 2 +- .../LambdaExpressionBuilderTest.cs | 3 +- test/RulesEngine.UnitTest/NestedRulesTest.cs | 141 +++++++++++++++++++++ test/RulesEngine.UnitTest/ScopedParamsTest.cs | 90 +++++++++++-- 8 files changed, 290 insertions(+), 38 deletions(-) create mode 100644 test/RulesEngine.UnitTest/NestedRulesTest.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index d55985b..783dd2e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/RulesEngine/Models/ReSettings.cs b/src/RulesEngine/Models/ReSettings.cs index 3dd689c..7658c24 100644 --- a/src/RulesEngine/Models/ReSettings.cs +++ b/src/RulesEngine/Models/ReSettings.cs @@ -44,6 +44,11 @@ namespace RulesEngine.Models /// public bool EnableScopedParams { get; set; } = true; + /// + /// Sets the mode for Nested rule execution, Default: All + /// + public NestedRuleExecutionMode NestedRuleExecutionMode { get; set; } = NestedRuleExecutionMode.All; + /// /// Enables Local params for rules /// @@ -53,4 +58,16 @@ namespace RulesEngine.Models set { EnableScopedParams = value; } } } + + public enum NestedRuleExecutionMode + { + /// + /// Excutes all nested rules + /// + All, + /// + /// Skips nested rules whose execution does not impact parent rule's result + /// + Performance + } } diff --git a/src/RulesEngine/Models/Rule.cs b/src/RulesEngine/Models/Rule.cs index 6fa9e77..03010eb 100644 --- a/src/RulesEngine/Models/Rule.cs +++ b/src/RulesEngine/Models/Rule.cs @@ -41,8 +41,8 @@ namespace RulesEngine.Models [JsonConverter(typeof(StringEnumConverter))] public RuleExpressionType RuleExpressionType { get; set; } = RuleExpressionType.LambdaExpression; - public List WorkflowRulesToInject { get; set; } - public List Rules { get; set; } + public IEnumerable WorkflowRulesToInject { get; set; } + public IEnumerable Rules { get; set; } public IEnumerable LocalParams { get; set; } public string Expression { get; set; } public Dictionary Actions { get; set; } diff --git a/src/RulesEngine/RuleCompiler.cs b/src/RulesEngine/RuleCompiler.cs index 5ee9730..907296d 100644 --- a/src/RulesEngine/RuleCompiler.cs +++ b/src/RulesEngine/RuleCompiler.cs @@ -186,33 +186,56 @@ namespace RulesEngine } return (paramArray) => { - var resultList = ruleFuncList.Select(fn => fn(paramArray)).ToList(); - Func isSuccess = (p) => ApplyOperation(resultList, operation); - var result = Helpers.ToResultTree(_reSettings, parentRule, resultList, isSuccess); + var (isSuccess, resultList) = ApplyOperation(paramArray, ruleFuncList, operation); + Func isSuccessFn = (p) => isSuccess; + var result = Helpers.ToResultTree(_reSettings, parentRule, resultList, isSuccessFn); return result(paramArray); }; } - private bool ApplyOperation(IEnumerable ruleResults, ExpressionType operation) + private (bool isSuccess ,IEnumerable result) ApplyOperation(RuleParameter[] paramArray,IEnumerable> ruleFuncList, ExpressionType operation) { - if (ruleResults?.Any() != true) + if (ruleFuncList?.Any() != true) { - return false; + return (false,new List()); } - switch (operation) - { - case ExpressionType.And: - case ExpressionType.AndAlso: - return ruleResults.All(r => r.IsSuccess); + var resultList = new List(); + 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 GetWrappedRuleFunc(Rule rule, RuleFunc 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); } diff --git a/src/RulesEngine/Validators/RuleValidator.cs b/src/RulesEngine/Validators/RuleValidator.cs index cc76d7f..2db62f0 100644 --- a/src/RulesEngine/Validators/RuleValidator.cs +++ b/src/RulesEngine/Validators/RuleValidator.cs @@ -43,7 +43,7 @@ namespace RulesEngine.Validators }); } - private bool BeValidRulesList(List rules) + private bool BeValidRulesList(IEnumerable rules) { if (rules?.Any() != true) return false; var validator = new RuleValidator(); diff --git a/test/RulesEngine.UnitTest/LambdaExpressionBuilderTest.cs b/test/RulesEngine.UnitTest/LambdaExpressionBuilderTest.cs index beed4cb..3e98ca6 100644 --- a/test/RulesEngine.UnitTest/LambdaExpressionBuilderTest.cs +++ b/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); diff --git a/test/RulesEngine.UnitTest/NestedRulesTest.cs b/test/RulesEngine.UnitTest/NestedRulesTest.cs new file mode 100644 index 0000000..99c049e --- /dev/null +++ b/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 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" + } + + } + } + } + }, + + }; + } + } +} diff --git a/test/RulesEngine.UnitTest/ScopedParamsTest.cs b/test/RulesEngine.UnitTest/ScopedParamsTest.cs index eef3e88..e3467fe 100644 --- a/test/RulesEngine.UnitTest/ScopedParamsTest.cs +++ b/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 { + new Rule { + + RuleName = "WithLocalParam", + LocalParams = new List { + new ScopedParam { + Name = "localParam1", + Expression = "input1.trueValue" + } + }, + Expression = "localParam1 == true" + } + } + }, + new WorkflowRules { WorkflowName = "GlobalParamsOnly", GlobalParams = new List { @@ -296,7 +339,7 @@ namespace RulesEngine.UnitTest new ScopedParam { Name = "localParam1", Expression = @"""world""" - } + } }, Rules = new List{ new Rule{ @@ -318,7 +361,38 @@ namespace RulesEngine.UnitTest } } - } + }, + new WorkflowRules { + WorkflowName = "LocalParamsOnlyWithComplexInput", + Rules = new List { + new Rule { + + RuleName = "WithLocalParam", + LocalParams = new List { + new ScopedParam { + Name = "localParam1", + Expression = "input1.Country.ToLower()" + } + }, + Expression = "localParam1 == \"hello\"" + } + } + }, + new WorkflowRules { + WorkflowName = "GlobalParamsOnlyWithComplexInput", + GlobalParams = new List { + new ScopedParam { + Name = "globalParam1", + Expression = "input1.Country.ToLower()" + } + }, + Rules = new List { + new Rule { + RuleName = "TrueTest", + Expression = "globalParam1 == \"hello\"" + } + } + }, }; } }