diff --git a/src/RulesEngine/RulesCache.cs b/src/RulesEngine/RulesCache.cs index 968d7b80..d8ab99ae 100644 --- a/src/RulesEngine/RulesCache.cs +++ b/src/RulesEngine/RulesCache.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using RulesEngine.HelperFunctions; @@ -41,6 +41,12 @@ public Func> GetGlobalParamsDelegate(string return _compiledGlobalParams.Get>>(compiledRuleKey); } + /// Removes any cached workflow-level global-params delegate for the specified compiled rule key. + public void RemoveGlobalParamsDelegate(string compiledRuleKey) + { + _compiledGlobalParams.Remove(compiledRuleKey); + } + /// Determines whether [contains workflow rules] [the specified workflow name]. /// Name of the workflow. @@ -150,7 +156,7 @@ public void Remove(string workflowName) { if (_workflow.TryRemove(workflowName, out var workflowObj)) { - var compiledKeysToRemove = _compileRules.GetKeys().Where(key => key.StartsWith(workflowName)).ToList(); + var compiledKeysToRemove = _compileRules.GetKeys().Where(key => key.StartsWith($"{workflowName}-", StringComparison.Ordinal)).ToList(); foreach (var key in compiledKeysToRemove) { _compileRules.Remove(key); diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index b31bb88c..ab7e2406 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. +// Copyright (c) Microsoft Corporation. // Licensed under the MIT License. using FluentValidation; @@ -157,7 +157,7 @@ public async ValueTask ExecuteActionWorkflowAsync(string workf sortedRuleParams.Sort((a, b) => string.Compare(a.Name, b.Name)); var sortedArr = sortedRuleParams.ToArray(); - // Compile the whole workflow once and reuse — was previously recompiled every call, + // Compile the whole workflow once and reuse — was previously recompiled every call, // which was the hot path the reporter of #471 saw. if (!RegisterRule(workflowName, sortedArr)) { @@ -422,13 +422,17 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams var globalParamsDelegate = _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParams, globalParamValues); _rulesCache.AddOrUpdateGlobalParamsDelegate(compileRulesKey, globalParamsDelegate); } + else + { + _rulesCache.RemoveGlobalParamsDelegate(compileRulesKey); + } var enabledRules = workflow.Rules.Where(c => c.Enabled).ToArray(); var compiledFuncs = new RuleFunc[enabledRules.Length]; // Parallel compilation helps only when: // - the user opted in, - // - they're not also on UseFastExpressionCompiler (which regresses ~3× under + // - they're not also on UseFastExpressionCompiler (which regresses ~3× under // parallel contention; FEC's internal locking serializes effort), and // - there are enough rules to amortize Parallel.For's scheduling cost. var shouldParallelize = _reSettings.EnableParallelRuleCompilation diff --git a/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs b/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs new file mode 100644 index 00000000..e092bb05 --- /dev/null +++ b/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs @@ -0,0 +1,82 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using RulesEngine.Models; +using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; +using System.Threading.Tasks; +using Xunit; + +namespace RulesEngine.UnitTest +{ + [Trait("Category", "Unit")] + [ExcludeFromCodeCoverage] + public class CacheRemoveAndStaleGlobalParamsTest + { + [Fact] + public async Task AddOrUpdateWorkflow_RemovingGlobalParams_ClearsStaleGlobalParamsDelegate() + { + var workflowV1 = new Workflow + { + WorkflowName = "DynamicGlobalsWorkflow", + GlobalParams = new List + { + new ScopedParam { Name = "bonus", Expression = "input1.Value * 2" } + }, + Rules = new List + { + new Rule { RuleName = "Rule1", Expression = "bonus > 10" } + } + }; + + var engine = new RulesEngine(new[] { workflowV1 }); + + var resV1 = await engine.ExecuteAllRulesAsync("DynamicGlobalsWorkflow", + new RuleParameter("input1", new { Value = 10 })); + Assert.True(resV1[0].IsSuccess); + Assert.True(resV1[0].Inputs.ContainsKey("bonus")); + + // Update workflow to remove GlobalParams completely. + var workflowV2 = new Workflow + { + WorkflowName = "DynamicGlobalsWorkflow", + GlobalParams = null, + Rules = new List + { + new Rule { RuleName = "Rule1", Expression = "input1.Value > 5" } + } + }; + + engine.AddOrUpdateWorkflow(workflowV2); + + var resV2 = await engine.ExecuteAllRulesAsync("DynamicGlobalsWorkflow", + new RuleParameter("input1", new { Value = 10 })); + Assert.True(resV2[0].IsSuccess); + Assert.False(resV2[0].Inputs.ContainsKey("bonus")); + } + + [Fact] + public void RulesCache_RemoveWorkflow_DoesNotEvictOtherWorkflowWithSameNamePrefix() + { + var cache = new RulesCache(new ReSettings()); + var wfOrder = new Workflow { WorkflowName = "Order" }; + var wfOrderApproval = new Workflow { WorkflowName = "OrderApproval" }; + + cache.AddOrUpdateWorkflows("Order", wfOrder); + cache.AddOrUpdateWorkflows("OrderApproval", wfOrderApproval); + + var dummyRules = new Dictionary>(); + cache.AddOrUpdateCompiledRule("Order-input1_Object", dummyRules); + cache.AddOrUpdateCompiledRule("OrderApproval-input1_Object", dummyRules); + + Assert.True(cache.AreCompiledRulesUpToDate("Order-input1_Object", "Order")); + Assert.True(cache.AreCompiledRulesUpToDate("OrderApproval-input1_Object", "OrderApproval")); + + cache.Remove("Order"); + + Assert.False(cache.AreCompiledRulesUpToDate("Order-input1_Object", "Order")); + // "OrderApproval" must still have its compiled rules cached and up-to-date. + Assert.True(cache.AreCompiledRulesUpToDate("OrderApproval-input1_Object", "OrderApproval")); + } + } +}