From 62e7533e19c59ff933086906f872b66fba0aa74e Mon Sep 17 00:00:00 2001 From: MBrekhof Date: Tue, 20 Oct 2026 18:38:47 +0200 Subject: [PATCH 1/3] Fix RulesCache.Remove prefix collision and clear stale GlobalParams on workflow update --- src/RulesEngine/RulesCache.cs | 10 ++- src/RulesEngine/RulesEngine.cs | 10 ++- .../CacheRemoveAndStaleGlobalParamsTest.cs | 82 +++++++++++++++++++ 3 files changed, 97 insertions(+), 5 deletions(-) create mode 100644 test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs 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")); + } + } +} From 652d86a8fceec0c0db574ec2cf571b17ddac87cd Mon Sep 17 00:00:00 2001 From: MBrekhof Date: Tue, 3 Nov 2026 20:56:05 +0100 Subject: [PATCH 2/3] Restore original UTF-8 BOM encoding of edited source files The previous commit re-saved the file without its BOM (and as cp1252 where comments held non-ASCII characters). Restore the original encoding so the diff only contains the intended change. Co-Authored-By: Claude Opus 5.5 --- src/RulesEngine/RulesCache.cs | 2 +- src/RulesEngine/RulesEngine.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/RulesEngine/RulesCache.cs b/src/RulesEngine/RulesCache.cs index d8ab99ae..e5444934 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; diff --git a/src/RulesEngine/RulesEngine.cs b/src/RulesEngine/RulesEngine.cs index ab7e2406..df86e16e 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)) { @@ -432,7 +432,7 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams // 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 From 4b7f3b2b27fa994dab6b017c7de379da5ea055f8 Mon Sep 17 00:00:00 2001 From: MBrekhof Date: Tue, 3 Nov 2026 21:07:32 +0100 Subject: [PATCH 3/3] Match the exact workflow name in RulesCache.Remove and keep globals in step with rules Compiled-rule keys are "{workflow}-{params}", and workflow names may contain "-", so a key prefix can't tell workflow "A" from "A-B": removing "A" also evicted the compiled rules of "A-B". Store the owning workflow name in each compiled-rules cache entry and remove only the entries whose name matches. Also update the workflow-level GlobalParams delegate together with the compiled rules: when the new GlobalParams fail to compile, the previous version's delegate is now dropped instead of still running next to the error results, and a rule compilation that throws no longer leaves the globals delegate one version ahead of the compiled rules. Co-Authored-By: Claude Opus 5.5 --- src/RulesEngine/RulesCache.cs | 15 ++-- src/RulesEngine/RulesEngine.cs | 27 ++++--- .../CacheRemoveAndStaleGlobalParamsTest.cs | 76 ++++++++++++++++++- 3 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/RulesEngine/RulesCache.cs b/src/RulesEngine/RulesCache.cs index e5444934..3516138b 100644 --- a/src/RulesEngine/RulesCache.cs +++ b/src/RulesEngine/RulesCache.cs @@ -73,11 +73,12 @@ public void AddOrUpdateWorkflows(string workflowName, Workflow rules) /// Adds the or update compiled rule. /// The compiled rule key. + /// The workflow the rules were compiled from. /// The compiled rule. - public void AddOrUpdateCompiledRule(string compiledRuleKey, IDictionary> compiledRule) + public void AddOrUpdateCompiledRule(string compiledRuleKey, string workflowName, IDictionary> compiledRule) { long ticks = DateTime.UtcNow.Ticks; - _compileRules.Set(compiledRuleKey,(compiledRule, ticks)); + _compileRules.Set(compiledRuleKey, (compiledRule, ticks, workflowName)); } /// Checks if the compiled rules are up-to-date. @@ -87,7 +88,7 @@ public void AddOrUpdateCompiledRule(string compiledRuleKey, IDictionarytrue if [compiled rules] is newer than the [workflow rules]; otherwise, false. public bool AreCompiledRulesUpToDate(string compiledRuleKey, string workflowName) { - if (_compileRules.TryGetValue(compiledRuleKey, out (IDictionary> rules, long tick) compiledRulesObj)) + if (_compileRules.TryGetValue(compiledRuleKey, out (IDictionary> rules, long tick, string workflowName) compiledRulesObj)) { if (_workflow.TryGetValue(workflowName, out (Workflow rules, long tick) WorkflowsObj)) { @@ -147,7 +148,7 @@ public Workflow GetWorkflow(string workflowName) /// CompiledRule. public IDictionary> GetCompiledRules(string compiledRulesKey) { - return _compileRules.Get<(IDictionary> rules, long tick)>(compiledRulesKey).rules; + return _compileRules.Get<(IDictionary> rules, long tick, string workflowName)>(compiledRulesKey).rules; } /// Removes the specified workflow name. @@ -156,7 +157,11 @@ public void Remove(string workflowName) { if (_workflow.TryRemove(workflowName, out var workflowObj)) { - var compiledKeysToRemove = _compileRules.GetKeys().Where(key => key.StartsWith($"{workflowName}-", StringComparison.Ordinal)).ToList(); + // Match on the stored workflow name: a key prefix can't tell workflow "A" from "A-B". + var compiledKeysToRemove = _compileRules.GetKeys() + .Where(key => _compileRules.TryGetValue(key, out (IDictionary> rules, long tick, string workflowName) entry) + && string.Equals(entry.workflowName, 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 df86e16e..b9ad7eae 100644 --- a/src/RulesEngine/RulesEngine.cs +++ b/src/RulesEngine/RulesEngine.cs @@ -411,21 +411,17 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams var msg = $"Error while compiling rule `{rule.RuleName}`: {ex.Message}"; dictFunc.Add(rule.RuleName, Helpers.ToRuleExceptionResult(_reSettings, rule, new RuleException(msg, ex))); } - _rulesCache.AddOrUpdateCompiledRule(compileRulesKey, dictFunc); + // Drop the previous version's globals delegate, or it keeps running next to these error results. + _rulesCache.RemoveGlobalParamsDelegate(compileRulesKey); + _rulesCache.AddOrUpdateCompiledRule(compileRulesKey, workflowName, dictFunc); return true; } var globalParamExp = new Lazy(() => globalParamValues); - if (globalParamValues.Length > 0) - { - var globalParamsDelegate = _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParams, globalParamValues); - _rulesCache.AddOrUpdateGlobalParamsDelegate(compileRulesKey, globalParamsDelegate); - } - else - { - _rulesCache.RemoveGlobalParamsDelegate(compileRulesKey); - } + var globalParamsDelegate = globalParamValues.Length > 0 + ? _ruleCompiler.CompileScopedParams(workflow.RuleExpressionType, ruleParams, globalParamValues) + : null; var enabledRules = workflow.Rules.Where(c => c.Enabled).ToArray(); var compiledFuncs = new RuleFunc[enabledRules.Length]; @@ -466,7 +462,16 @@ private bool RegisterRule(string workflowName, params RuleParameter[] ruleParams dictFunc.Add(enabledRules[i].RuleName, compiledFuncs[i]); } - _rulesCache.AddOrUpdateCompiledRule(compileRulesKey, dictFunc); + // Update the globals delegate only once the rules compiled, so both caches hold the same version. + if (globalParamsDelegate != null) + { + _rulesCache.AddOrUpdateGlobalParamsDelegate(compileRulesKey, globalParamsDelegate); + } + else + { + _rulesCache.RemoveGlobalParamsDelegate(compileRulesKey); + } + _rulesCache.AddOrUpdateCompiledRule(compileRulesKey, workflowName, dictFunc); return true; } else diff --git a/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs b/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs index e092bb05..af7ea1c9 100644 --- a/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs +++ b/test/RulesEngine.UnitTest/CacheRemoveAndStaleGlobalParamsTest.cs @@ -66,8 +66,8 @@ public void RulesCache_RemoveWorkflow_DoesNotEvictOtherWorkflowWithSameNamePrefi cache.AddOrUpdateWorkflows("OrderApproval", wfOrderApproval); var dummyRules = new Dictionary>(); - cache.AddOrUpdateCompiledRule("Order-input1_Object", dummyRules); - cache.AddOrUpdateCompiledRule("OrderApproval-input1_Object", dummyRules); + cache.AddOrUpdateCompiledRule("Order-input1_Object", "Order", dummyRules); + cache.AddOrUpdateCompiledRule("OrderApproval-input1_Object", "OrderApproval", dummyRules); Assert.True(cache.AreCompiledRulesUpToDate("Order-input1_Object", "Order")); Assert.True(cache.AreCompiledRulesUpToDate("OrderApproval-input1_Object", "OrderApproval")); @@ -78,5 +78,77 @@ public void RulesCache_RemoveWorkflow_DoesNotEvictOtherWorkflowWithSameNamePrefi // "OrderApproval" must still have its compiled rules cached and up-to-date. Assert.True(cache.AreCompiledRulesUpToDate("OrderApproval-input1_Object", "OrderApproval")); } + + [Fact] + public void RulesCache_RemoveWorkflow_DoesNotEvictWorkflowNamedWithRemovedNameAndHyphen() + { + // Compiled keys are "{workflow}-{params}", so "A-B-input1_Object" also starts with "A-". + var cache = new RulesCache(new ReSettings()); + cache.AddOrUpdateWorkflows("A", new Workflow { WorkflowName = "A" }); + cache.AddOrUpdateWorkflows("A-B", new Workflow { WorkflowName = "A-B" }); + + var dummyRules = new Dictionary>(); + cache.AddOrUpdateCompiledRule("A-input1_Object", "A", dummyRules); + cache.AddOrUpdateCompiledRule("A-input1_Object-input2_Object", "A", dummyRules); + cache.AddOrUpdateCompiledRule("A-B-input1_Object", "A-B", dummyRules); + cache.AddOrUpdateGlobalParamsDelegate("A-input1_Object", _ => new Dictionary()); + cache.AddOrUpdateGlobalParamsDelegate("A-B-input1_Object", _ => new Dictionary()); + + cache.Remove("A"); + + Assert.Null(cache.GetCompiledRules("A-input1_Object")); + Assert.Null(cache.GetCompiledRules("A-input1_Object-input2_Object")); + Assert.Null(cache.GetGlobalParamsDelegate("A-input1_Object")); + Assert.NotNull(cache.GetCompiledRules("A-B-input1_Object")); + Assert.NotNull(cache.GetGlobalParamsDelegate("A-B-input1_Object")); + Assert.True(cache.AreCompiledRulesUpToDate("A-B-input1_Object", "A-B")); + + cache.Remove("A-B"); + + Assert.Null(cache.GetCompiledRules("A-B-input1_Object")); + Assert.Null(cache.GetGlobalParamsDelegate("A-B-input1_Object")); + } + + [Fact] + public async Task AddOrUpdateWorkflow_GlobalParamsFailingToCompile_ClearsStaleGlobalParamsDelegate() + { + var workflowV1 = new Workflow + { + WorkflowName = "BrokenGlobalsWorkflow", + 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("BrokenGlobalsWorkflow", + new RuleParameter("input1", new { Value = 10 })); + Assert.True(resV1[0].Inputs.ContainsKey("bonus")); + + // V2's global param no longer compiles; V1's compiled delegate must not keep running. + var workflowV2 = new Workflow + { + WorkflowName = "BrokenGlobalsWorkflow", + GlobalParams = new List + { + new ScopedParam { Name = "other", Expression = "input1.DoesNotExist * 2" } + }, + Rules = new List + { + new Rule { RuleName = "Rule1", Expression = "other > 10" } + } + }; + engine.AddOrUpdateWorkflow(workflowV2); + + var resV2 = await engine.ExecuteAllRulesAsync("BrokenGlobalsWorkflow", + new RuleParameter("input1", new { Value = 10 })); + Assert.False(resV2[0].IsSuccess); + Assert.False(resV2[0].Inputs.ContainsKey("bonus")); + } } }