From 2079f88c8f8888d543e799c882663da12b85af5f Mon Sep 17 00:00:00 2001 From: wadii Date: Thu, 7 May 2026 10:26:11 +0200 Subject: [PATCH] fix: respect rule type for sub-rules in segment evaluation Sub-rules were always evaluated with ALL logic, ignoring the parent rule's type field (ANY/ALL/NONE). Now uses type-aware matching for sub-rules, matching the existing conditions behavior. Bumps engine-test-data from v3.5.0 to v3.7.0 which includes test cases for this fix. Equivalent fix to flagsmith-engine#313. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitmodules | 2 +- .../flagengine/segments/SegmentEvaluator.java | 23 +++++++++++++++++-- .../com/flagsmith/flagengine/enginetestdata | 2 +- .../unit/segments/SegmentEvaluatorTest.java | 2 +- 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.gitmodules b/.gitmodules index 8cfdf100..dcaa9e95 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "src/test/java/com/flagsmith/flagengine/enginetestdata"] path = src/test/java/com/flagsmith/flagengine/enginetestdata url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.5.0 \ No newline at end of file + branch = v3.7.0 \ No newline at end of file diff --git a/src/main/java/com/flagsmith/flagengine/segments/SegmentEvaluator.java b/src/main/java/com/flagsmith/flagengine/segments/SegmentEvaluator.java index a38ce3aa..cbfb741e 100644 --- a/src/main/java/com/flagsmith/flagengine/segments/SegmentEvaluator.java +++ b/src/main/java/com/flagsmith/flagengine/segments/SegmentEvaluator.java @@ -71,8 +71,27 @@ private static Boolean contextMatchesRule(EvaluationContext context, SegmentRule } } - return isMatch && rule.getRules().stream() - .allMatch((subRule) -> contextMatchesRule(context, subRule, segmentKey)); + if (!isMatch) { + return false; + } + + List subRules = rule.getRules(); + if (subRules.isEmpty()) { + return true; + } + + Predicate subRulePredicate = (subRule) -> contextMatchesRule( + context, subRule, segmentKey); + switch (rule.getType()) { + case ALL: + return subRules.stream().allMatch(subRulePredicate); + case ANY: + return subRules.stream().anyMatch(subRulePredicate); + case NONE: + return subRules.stream().noneMatch(subRulePredicate); + default: + return false; + } } private static Boolean contextMatchesCondition( diff --git a/src/test/java/com/flagsmith/flagengine/enginetestdata b/src/test/java/com/flagsmith/flagengine/enginetestdata index 7840a134..4b29dc77 160000 --- a/src/test/java/com/flagsmith/flagengine/enginetestdata +++ b/src/test/java/com/flagsmith/flagengine/enginetestdata @@ -1 +1 @@ -Subproject commit 7840a1349b601df3b6b4a089f40864f659801afb +Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f diff --git a/src/test/java/com/flagsmith/flagengine/unit/segments/SegmentEvaluatorTest.java b/src/test/java/com/flagsmith/flagengine/unit/segments/SegmentEvaluatorTest.java index f7b23fd3..a11007ec 100644 --- a/src/test/java/com/flagsmith/flagengine/unit/segments/SegmentEvaluatorTest.java +++ b/src/test/java/com/flagsmith/flagengine/unit/segments/SegmentEvaluatorTest.java @@ -38,7 +38,7 @@ private static Stream identitiesInSegments() { Arguments.of(segmentMultipleConditionsAny(), oneIdentityTrait(), Boolean.TRUE), Arguments.of(segmentMultipleConditionsAny(), twoIdentityTraits(), Boolean.TRUE), Arguments.of(segmentNestedRules(), emptyIdentityTraits(), Boolean.FALSE), - Arguments.of(segmentNestedRules(), oneIdentityTrait(), Boolean.FALSE), + Arguments.of(segmentNestedRules(), oneIdentityTrait(), Boolean.TRUE), Arguments.of(segmentNestedRules(), threeIdentityTraits(), Boolean.TRUE), Arguments.of(segmentConditionsAndNestedRules(), emptyIdentityTraits(), Boolean.FALSE), Arguments.of(segmentConditionsAndNestedRules(), oneIdentityTrait(), Boolean.FALSE),