From b98233457e420fdfb89cb1fdcd05b2332d70e376 Mon Sep 17 00:00:00 2001 From: maliming Date: Wed, 13 May 2026 11:13:28 +0800 Subject: [PATCH] Fix SimpleStateCheckerManager overwriting result between batch checkers When a state has multiple batch checkers (e.g. RequirePermissions plus RequireFeatures), the per-state result was overwritten on every checker pass, so a later 'true' silently masked an earlier 'false'. AND-combine the results to require all checkers to pass. --- .../SimpleStateCheckerManager.cs | 2 +- ...pleStateChecker_AndCombineResults_Tests.cs | 101 ++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 framework/test/Volo.Abp.Core.Tests/Volo/Abp/SimpleStateChecking/SimpleStateChecker_AndCombineResults_Tests.cs diff --git a/framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerManager.cs b/framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerManager.cs index 3d01c3c48d..ffc02d3207 100644 --- a/framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerManager.cs +++ b/framework/src/Volo.Abp.Core/Volo/Abp/SimpleStateChecking/SimpleStateCheckerManager.cs @@ -46,7 +46,7 @@ public class SimpleStateCheckerManager : ISimpleStateCheckerManager !x)) diff --git a/framework/test/Volo.Abp.Core.Tests/Volo/Abp/SimpleStateChecking/SimpleStateChecker_AndCombineResults_Tests.cs b/framework/test/Volo.Abp.Core.Tests/Volo/Abp/SimpleStateChecking/SimpleStateChecker_AndCombineResults_Tests.cs new file mode 100644 index 0000000000..6fdc54b940 --- /dev/null +++ b/framework/test/Volo.Abp.Core.Tests/Volo/Abp/SimpleStateChecking/SimpleStateChecker_AndCombineResults_Tests.cs @@ -0,0 +1,101 @@ +using System.Threading.Tasks; +using Shouldly; +using Volo.Abp.DependencyInjection; +using Xunit; + +namespace Volo.Abp.SimpleStateChecking; + +public class SimpleStateChecker_AndCombineResults_Tests : SimpleStateCheckerTestBase +{ + [Fact] + public async Task False_Result_From_One_Batch_Checker_Should_Not_Be_Overwritten_By_Later_True_Checker() + { + // Regression test for the bug fixed in SimpleStateCheckerManager.IsEnabledAsync: + // when a state had multiple batch checkers the manager used to OVERWRITE + // result[x.Key] on each checker, so a later "true" silently masked an earlier "false". + // The fix AND-combines results. + // + // To make sure we actually hit the second batch checker (and not the early-return + // `result.Values.All(x => !x)` short-circuit), we use two states: + // - state1: [falseChecker, trueChecker] - the target case + // - state2: [trueChecker] - keeps result.Values not all-false + // so the loop continues to the next checker + + var falseChecker = new AlwaysFalseBatchStateChecker(); + var trueChecker = new AlwaysTrueBatchStateChecker(); + + var state1 = new MyStateEntity(); + state1.AddSimpleStateChecker(falseChecker); + state1.AddSimpleStateChecker(trueChecker); + + var state2 = new MyStateEntity(); + state2.AddSimpleStateChecker(trueChecker); + + var result = await SimpleStateCheckerManager.IsEnabledAsync(new[] { state1, state2 }); + + result[state1].ShouldBeFalse(); // before the fix this was True (bug) + result[state2].ShouldBeTrue(); + } + + [Fact] + public async Task True_Result_From_One_Batch_Checker_Should_Be_AND_Combined_With_Later_False_Checker() + { + // Reverse order: true first then false. Add a second always-true state to keep + // result.Values not all-false after both checkers ran. + var trueChecker = new AlwaysTrueBatchStateChecker(); + var falseChecker = new AlwaysFalseBatchStateChecker(); + + var state1 = new MyStateEntity(); + state1.AddSimpleStateChecker(trueChecker); + state1.AddSimpleStateChecker(falseChecker); + + var state2 = new MyStateEntity(); + state2.AddSimpleStateChecker(trueChecker); + + var result = await SimpleStateCheckerManager.IsEnabledAsync(new[] { state1, state2 }); + + result[state1].ShouldBeFalse(); + result[state2].ShouldBeTrue(); + } + + [Fact] + public async Task All_True_Batch_Checkers_Should_Produce_True() + { + var trueChecker1 = new AlwaysTrueBatchStateChecker(); + var trueChecker2 = new AlwaysTrueBatchStateChecker(); + + var state = new MyStateEntity(); + state.AddSimpleStateChecker(trueChecker1); + state.AddSimpleStateChecker(trueChecker2); + + var result = await SimpleStateCheckerManager.IsEnabledAsync(new[] { state }); + + result[state].ShouldBeTrue(); + } + + public class AlwaysFalseBatchStateChecker : SimpleBatchStateCheckerBase, ITransientDependency + { + public override Task> IsEnabledAsync(SimpleBatchStateCheckerContext context) + { + var result = new SimpleStateCheckerResult(context.States); + foreach (var x in result) + { + result[x.Key] = false; + } + return Task.FromResult(result); + } + } + + public class AlwaysTrueBatchStateChecker : SimpleBatchStateCheckerBase, ITransientDependency + { + public override Task> IsEnabledAsync(SimpleBatchStateCheckerContext context) + { + var result = new SimpleStateCheckerResult(context.States); + foreach (var x in result) + { + result[x.Key] = true; + } + return Task.FromResult(result); + } + } +}