From 212cb1dd00bed11631c064a6bdf6f68855fed621 Mon Sep 17 00:00:00 2001 From: kaviska Date: Fri, 10 Jul 2026 11:53:33 +0530 Subject: [PATCH 1/7] Add failed fields to rule evaluation result Track and expose the fields that caused a rule to fail evaluation. - RuleEvaluationResult: add a failedFields list, a constructor accepting it, and a getFailedFields() accessor (empty when the rule is satisfied) - RuleEvaluator: collect the field names of failing expressions per OR branch, clearing them when a branch passes, and expose them via getFailedFields() - RuleEvaluationServiceImpl: pass the evaluator's failed fields into the result --- .../api/model/RuleEvaluationResult.java | 19 +++++++++++++++ .../impl/RuleEvaluationServiceImpl.java | 2 +- .../internal/service/impl/RuleEvaluator.java | 23 +++++++++++++++++-- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java index 022550c0d7ca..12efec21931d 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java @@ -18,6 +18,9 @@ package org.wso2.carbon.identity.rule.evaluation.api.model; +import java.util.Collections; +import java.util.List; + /** * Rule evaluation result. * This class is used to represent the result of a rule evaluation. @@ -26,11 +29,18 @@ public class RuleEvaluationResult { private final String ruleId; private final boolean ruleSatisfied; + private final List failedFields; public RuleEvaluationResult(String ruleId, boolean ruleSatisfied) { + this(ruleId, ruleSatisfied, Collections.emptyList()); + } + + public RuleEvaluationResult(String ruleId, boolean ruleSatisfied, List failedFields) { + this.ruleId = ruleId; this.ruleSatisfied = ruleSatisfied; + this.failedFields = failedFields != null ? failedFields : Collections.emptyList(); } public String getRuleId() { @@ -42,4 +52,13 @@ public boolean isRuleSatisfied() { return ruleSatisfied; } + + /** + * Returns the list of fields that failed evaluation. + * Empty when the rule is satisfied. + */ + public List getFailedFields() { + + return Collections.unmodifiableList(failedFields); + } } diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java index 854f1bc36c60..ae464846c0f3 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java @@ -79,7 +79,7 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str boolean evaluationStatus = ruleEvaluator.evaluate(rule, evaluationData); LOG.debug("Evaluated rule: " + rule.getId() + " to: " + evaluationStatus + "."); - return new RuleEvaluationResult(ruleId, evaluationStatus); + return new RuleEvaluationResult(ruleId, evaluationStatus, ruleEvaluator.getFailedFields()); } private Map getEvaluationData(String ruleId, FlowContext flowContext, diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java index 3aa2b4b03bdf..1cc4795194c1 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java @@ -28,6 +28,8 @@ import org.wso2.carbon.identity.rule.management.api.model.ORCombinedRule; import org.wso2.carbon.identity.rule.management.api.model.Rule; +import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -46,6 +48,7 @@ public class RuleEvaluator { private static final Log LOG = LogFactory.getLog(RuleEvaluator.class); private final OperatorRegistry operatorRegistry; + private final List failedFields = new ArrayList<>(); // Operators private static final String EQUALS = "equals"; @@ -71,22 +74,38 @@ public boolean evaluate(Rule rule, Map evaluationData) throw return evaluateORCombinedRule(orRule, evaluationData); } + /** + * Returns the list of fields that failed evaluation for the last evaluated rule. + * Empty when the rule is satisfied. + * + * @return List of failed field names. + */ + public List getFailedFields() { + + return Collections.unmodifiableList(failedFields); + } + private boolean evaluateORCombinedRule(ORCombinedRule orRule, Map evaluationData) throws RuleEvaluationException { for (ANDCombinedRule andRule : orRule.getRules()) { - if (evaluateANDCombinedRule(andRule, evaluationData)) { + List branchFailedFields = new ArrayList<>(); + if (evaluateANDCombinedRule(andRule, evaluationData, branchFailedFields)) { + failedFields.clear(); return true; // If any ANDCombinedRule evaluates to true, the ORCombinedRule passes } + failedFields.addAll(branchFailedFields); } return false; // If none of the ANDCombinedRules pass, the ORCombinedRule fails } - private boolean evaluateANDCombinedRule(ANDCombinedRule andRule, Map evaluationData) + private boolean evaluateANDCombinedRule(ANDCombinedRule andRule, Map evaluationData, + List branchFailedFields) throws RuleEvaluationException { for (Expression expression : andRule.getExpressions()) { if (!evaluateExpression(expression, evaluationData)) { + branchFailedFields.add(expression.getField()); return false; // If any expression fails, the ANDCombinedRule fails } } From 279401b5fb85b6a52521632c3eda707c61e88524 Mon Sep 17 00:00:00 2001 From: kaviska Date: Fri, 10 Jul 2026 14:23:40 +0530 Subject: [PATCH 2/7] Add tests for rule evaluation failed fields Assert that getFailedFields() reports the failing field when a rule fails, and is empty when the rule passes. --- .../evaluation/core/RuleEvaluatorTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java index 3fa9b14d5c50..0111042b8820 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java @@ -60,6 +60,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.when; +import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; @@ -177,6 +178,29 @@ public void testEvaluateFieldValueNotFound() throws Exception { Collections.emptyMap()); } + @Test + public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { + + boolean result = ruleEvaluator.evaluate( + createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), + createEvaluationData("testApp", "client-credentials")); + + assertFalse(result); + // The rule fails on the first failing expression (application), so it is reported as a failed field. + assertEquals(ruleEvaluator.getFailedFields(), Collections.singletonList("application")); + } + + @Test + public void testFailedFieldsEmptyWhenRulePasses() throws Exception { + + boolean result = ruleEvaluator.evaluate( + createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), + createEvaluationData("testapp", "authorization_code")); + + assertTrue(result); + assertTrue(ruleEvaluator.getFailedFields().isEmpty()); + } + private Rule createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes() throws Exception { RuleBuilder ruleBuilder = RuleBuilder.create(FlowType.PRE_ISSUE_ACCESS_TOKEN, "tenant1"); From 7e1b00f7b6a0c8a10e74bea190f828bd566eca1d Mon Sep 17 00:00:00 2001 From: kaviska Date: Fri, 10 Jul 2026 18:12:17 +0530 Subject: [PATCH 3/7] Make rule evaluator stateless and return failed fields in the result Address review feedback on the failed fields feature. - RuleEvaluator: remove the shared failedFields instance field, which was unsafe under concurrent evaluation, and add an evaluate(ruleId, rule, data) overload that returns a RuleEvaluationResult carrying the failed fields. The boolean evaluate(rule, data) delegates to it. - RuleEvaluationServiceImpl: use the new evaluate overload and return its result directly, so callers read the failed fields from the returned object. - RuleEvaluationResult: deprecate the two-arg constructor and keep the failed fields as-is (null when not set) so callers can distinguish 'not computed' from 'no failed fields'. - Update tests to read the failed fields from the returned result. --- .../api/model/RuleEvaluationResult.java | 19 +++++++++++--- .../impl/RuleEvaluationServiceImpl.java | 8 +++--- .../internal/service/impl/RuleEvaluator.java | 25 ++++++++++++------- .../evaluation/core/RuleEvaluatorTest.java | 13 +++++----- 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java index 12efec21931d..fbf67730a12b 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/api/model/RuleEvaluationResult.java @@ -31,16 +31,25 @@ public class RuleEvaluationResult { private final boolean ruleSatisfied; private final List failedFields; + /** + * Creates a result without setting the failed fields. + * + * @deprecated Use {@link #RuleEvaluationResult(String, boolean, List)} which also captures the failed + * fields. This constructor leaves the failed fields unset ({@code null}), so callers cannot + * distinguish "no fields failed" from "failed fields were not computed". + */ + @Deprecated public RuleEvaluationResult(String ruleId, boolean ruleSatisfied) { - this(ruleId, ruleSatisfied, Collections.emptyList()); + this(ruleId, ruleSatisfied, null); } public RuleEvaluationResult(String ruleId, boolean ruleSatisfied, List failedFields) { this.ruleId = ruleId; this.ruleSatisfied = ruleSatisfied; - this.failedFields = failedFields != null ? failedFields : Collections.emptyList(); + // Keep as-is: null means the failed fields were not set, which is different from an empty list. + this.failedFields = failedFields; } public String getRuleId() { @@ -54,11 +63,13 @@ public boolean isRuleSatisfied() { } /** - * Returns the list of fields that failed evaluation. + * Returns the list of fields that failed evaluation, or {@code null} if the failed fields were not set. * Empty when the rule is satisfied. + * + * @return Unmodifiable list of failed field names, or {@code null} if not set. */ public List getFailedFields() { - return Collections.unmodifiableList(failedFields); + return failedFields == null ? null : Collections.unmodifiableList(failedFields); } } diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java index ae464846c0f3..8566b381c530 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java @@ -62,7 +62,7 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str if (!rule.isActive()) { LOG.debug("Rule: " + rule.getId() + " is inactive. Skip evaluation of rule."); - return new RuleEvaluationResult(ruleId, false); + return new RuleEvaluationResult(ruleId, false, null); } LOG.debug("Starting to evaluate rule: " + rule.getId() + "."); @@ -76,10 +76,10 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str RuleEvaluator ruleEvaluator = new RuleEvaluator(RuleEvaluationComponentServiceHolder.getInstance() .getOperatorRegistry()); - boolean evaluationStatus = ruleEvaluator.evaluate(rule, evaluationData); - LOG.debug("Evaluated rule: " + rule.getId() + " to: " + evaluationStatus + "."); + RuleEvaluationResult result = ruleEvaluator.evaluate(ruleId, rule, evaluationData); + LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); - return new RuleEvaluationResult(ruleId, evaluationStatus, ruleEvaluator.getFailedFields()); + return result; } private Map getEvaluationData(String ruleId, FlowContext flowContext, diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java index 1cc4795194c1..a3755e610b29 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java @@ -23,6 +23,7 @@ import org.wso2.carbon.identity.rule.evaluation.api.exception.RuleEvaluationException; import org.wso2.carbon.identity.rule.evaluation.api.model.FieldValue; import org.wso2.carbon.identity.rule.evaluation.api.model.Operator; +import org.wso2.carbon.identity.rule.evaluation.api.model.RuleEvaluationResult; import org.wso2.carbon.identity.rule.management.api.model.ANDCombinedRule; import org.wso2.carbon.identity.rule.management.api.model.Expression; import org.wso2.carbon.identity.rule.management.api.model.ORCombinedRule; @@ -48,7 +49,6 @@ public class RuleEvaluator { private static final Log LOG = LogFactory.getLog(RuleEvaluator.class); private final OperatorRegistry operatorRegistry; - private final List failedFields = new ArrayList<>(); // Operators private static final String EQUALS = "equals"; @@ -70,22 +70,29 @@ public RuleEvaluator(OperatorRegistry operatorRegistry) { */ public boolean evaluate(Rule rule, Map evaluationData) throws RuleEvaluationException { - ORCombinedRule orRule = (ORCombinedRule) rule; - return evaluateORCombinedRule(orRule, evaluationData); + return evaluate(null, rule, evaluationData).isRuleSatisfied(); } /** - * Returns the list of fields that failed evaluation for the last evaluated rule. - * Empty when the rule is satisfied. + * Evaluate a given rule and return the result, including the fields that failed evaluation. * - * @return List of failed field names. + * @param ruleId Rule ID. + * @param rule Rule to evaluate. + * @param evaluationData Evaluation data. + * @return Rule evaluation result with the satisfied status and the failed fields. + * @throws RuleEvaluationException If an error occurs while evaluating the rule. */ - public List getFailedFields() { + public RuleEvaluationResult evaluate(String ruleId, Rule rule, Map evaluationData) + throws RuleEvaluationException { - return Collections.unmodifiableList(failedFields); + List failedFields = new ArrayList<>(); + ORCombinedRule orRule = (ORCombinedRule) rule; + boolean ruleSatisfied = evaluateORCombinedRule(orRule, evaluationData, failedFields); + return new RuleEvaluationResult(ruleId, ruleSatisfied, failedFields); } - private boolean evaluateORCombinedRule(ORCombinedRule orRule, Map evaluationData) + private boolean evaluateORCombinedRule(ORCombinedRule orRule, Map evaluationData, + List failedFields) throws RuleEvaluationException { for (ANDCombinedRule andRule : orRule.getRules()) { diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java index 0111042b8820..11cda68d7054 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java @@ -26,6 +26,7 @@ import org.testng.annotations.Test; import org.wso2.carbon.identity.rule.evaluation.api.exception.RuleEvaluationException; import org.wso2.carbon.identity.rule.evaluation.api.model.FieldValue; +import org.wso2.carbon.identity.rule.evaluation.api.model.RuleEvaluationResult; import org.wso2.carbon.identity.rule.evaluation.api.model.ValueType; import org.wso2.carbon.identity.rule.evaluation.internal.component.RuleEvaluationComponentServiceHolder; import org.wso2.carbon.identity.rule.evaluation.internal.service.impl.OperatorRegistry; @@ -181,24 +182,24 @@ public void testEvaluateFieldValueNotFound() throws Exception { @Test public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { - boolean result = ruleEvaluator.evaluate( + RuleEvaluationResult result = ruleEvaluator.evaluate("rule1", createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testApp", "client-credentials")); - assertFalse(result); + assertFalse(result.isRuleSatisfied()); // The rule fails on the first failing expression (application), so it is reported as a failed field. - assertEquals(ruleEvaluator.getFailedFields(), Collections.singletonList("application")); + assertEquals(result.getFailedFields(), Collections.singletonList("application")); } @Test public void testFailedFieldsEmptyWhenRulePasses() throws Exception { - boolean result = ruleEvaluator.evaluate( + RuleEvaluationResult result = ruleEvaluator.evaluate("rule1", createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testapp", "authorization_code")); - assertTrue(result); - assertTrue(ruleEvaluator.getFailedFields().isEmpty()); + assertTrue(result.isRuleSatisfied()); + assertTrue(result.getFailedFields().isEmpty()); } private Rule createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes() throws Exception { From 386cb3ead0933346cf1e5ff4e187105b01f43e91 Mon Sep 17 00:00:00 2001 From: kaviska Date: Fri, 10 Jul 2026 18:56:50 +0530 Subject: [PATCH 4/7] Guard concatenated debug log with isDebugEnabled in RuleEvaluationServiceImpl --- .../internal/service/impl/RuleEvaluationServiceImpl.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java index 8566b381c530..92ee04f08ebf 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java @@ -77,7 +77,9 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str RuleEvaluator ruleEvaluator = new RuleEvaluator(RuleEvaluationComponentServiceHolder.getInstance() .getOperatorRegistry()); RuleEvaluationResult result = ruleEvaluator.evaluate(ruleId, rule, evaluationData); - LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); + if (LOG.isDebugEnabled()) { + LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); + } return result; } From 9308d74bbf1bb0d85da3f32104547718d863a2ad Mon Sep 17 00:00:00 2001 From: kaviska Date: Sat, 11 Jul 2026 12:13:54 +0530 Subject: [PATCH 5/7] Add evaluateResult method returning RuleEvaluationResult Replace the ruleId-overloaded evaluate with a distinctly named evaluateResult(Rule, Map) method that returns the full result. The rule id is derived from rule.getId(), so the extra ruleId parameter is no longer needed. The existing boolean evaluate(Rule, Map) is unchanged and delegates to evaluateResult, keeping the change purely additive. --- .../internal/service/impl/RuleEvaluationServiceImpl.java | 2 +- .../evaluation/internal/service/impl/RuleEvaluator.java | 7 +++---- .../identity/rule/evaluation/core/RuleEvaluatorTest.java | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java index 92ee04f08ebf..d117b237387f 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java @@ -76,7 +76,7 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str RuleEvaluator ruleEvaluator = new RuleEvaluator(RuleEvaluationComponentServiceHolder.getInstance() .getOperatorRegistry()); - RuleEvaluationResult result = ruleEvaluator.evaluate(ruleId, rule, evaluationData); + RuleEvaluationResult result = ruleEvaluator.evaluateResult(rule, evaluationData); if (LOG.isDebugEnabled()) { LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); } diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java index a3755e610b29..aedc52c74544 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java @@ -70,25 +70,24 @@ public RuleEvaluator(OperatorRegistry operatorRegistry) { */ public boolean evaluate(Rule rule, Map evaluationData) throws RuleEvaluationException { - return evaluate(null, rule, evaluationData).isRuleSatisfied(); + return evaluateResult(rule, evaluationData).isRuleSatisfied(); } /** * Evaluate a given rule and return the result, including the fields that failed evaluation. * - * @param ruleId Rule ID. * @param rule Rule to evaluate. * @param evaluationData Evaluation data. * @return Rule evaluation result with the satisfied status and the failed fields. * @throws RuleEvaluationException If an error occurs while evaluating the rule. */ - public RuleEvaluationResult evaluate(String ruleId, Rule rule, Map evaluationData) + public RuleEvaluationResult evaluateResult(Rule rule, Map evaluationData) throws RuleEvaluationException { List failedFields = new ArrayList<>(); ORCombinedRule orRule = (ORCombinedRule) rule; boolean ruleSatisfied = evaluateORCombinedRule(orRule, evaluationData, failedFields); - return new RuleEvaluationResult(ruleId, ruleSatisfied, failedFields); + return new RuleEvaluationResult(rule.getId(), ruleSatisfied, failedFields); } private boolean evaluateORCombinedRule(ORCombinedRule orRule, Map evaluationData, diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java index 11cda68d7054..1dbf501255da 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java @@ -182,7 +182,7 @@ public void testEvaluateFieldValueNotFound() throws Exception { @Test public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { - RuleEvaluationResult result = ruleEvaluator.evaluate("rule1", + RuleEvaluationResult result = ruleEvaluator.evaluateResult( createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testApp", "client-credentials")); @@ -194,7 +194,7 @@ public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { @Test public void testFailedFieldsEmptyWhenRulePasses() throws Exception { - RuleEvaluationResult result = ruleEvaluator.evaluate("rule1", + RuleEvaluationResult result = ruleEvaluator.evaluateResult( createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testapp", "authorization_code")); From 40c26bd8d0e25221d119b679571c41b407850e4a Mon Sep 17 00:00:00 2001 From: kaviska Date: Tue, 14 Jul 2026 17:07:19 +0530 Subject: [PATCH 6/7] Return RuleEvaluationResult from RuleEvaluator.evaluate RuleEvaluator is an internal class that is excluded from Export-Package, so it is not reachable outside the bundle and its only caller is RuleEvaluationServiceImpl. Change the existing evaluate(Rule, Map) to return RuleEvaluationResult instead of a boolean, and drop the extra method, leaving a single entry point. Callers that need the boolean use isRuleSatisfied(). --- .../service/impl/RuleEvaluationServiceImpl.java | 2 +- .../internal/service/impl/RuleEvaluator.java | 15 +-------------- .../rule/evaluation/core/RuleEvaluatorTest.java | 6 +++--- 3 files changed, 5 insertions(+), 18 deletions(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java index d117b237387f..372ac03d79da 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluationServiceImpl.java @@ -76,7 +76,7 @@ public RuleEvaluationResult evaluate(String ruleId, FlowContext flowContext, Str RuleEvaluator ruleEvaluator = new RuleEvaluator(RuleEvaluationComponentServiceHolder.getInstance() .getOperatorRegistry()); - RuleEvaluationResult result = ruleEvaluator.evaluateResult(rule, evaluationData); + RuleEvaluationResult result = ruleEvaluator.evaluate(rule, evaluationData); if (LOG.isDebugEnabled()) { LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); } diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java index aedc52c74544..885013d40ec5 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java @@ -60,19 +60,6 @@ public RuleEvaluator(OperatorRegistry operatorRegistry) { this.operatorRegistry = operatorRegistry; } - /** - * Evaluate a given rule. - * - * @param rule Rule to evaluate. - * @param evaluationData Evaluation data. - * @return Evaluation result. - * @throws RuleEvaluationException If an error occurs while evaluating the rule. - */ - public boolean evaluate(Rule rule, Map evaluationData) throws RuleEvaluationException { - - return evaluateResult(rule, evaluationData).isRuleSatisfied(); - } - /** * Evaluate a given rule and return the result, including the fields that failed evaluation. * @@ -81,7 +68,7 @@ public boolean evaluate(Rule rule, Map evaluationData) throw * @return Rule evaluation result with the satisfied status and the failed fields. * @throws RuleEvaluationException If an error occurs while evaluating the rule. */ - public RuleEvaluationResult evaluateResult(Rule rule, Map evaluationData) + public RuleEvaluationResult evaluate(Rule rule, Map evaluationData) throws RuleEvaluationException { List failedFields = new ArrayList<>(); diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java index 1dbf501255da..0e991ce13359 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/test/java/org/wso2/carbon/identity/rule/evaluation/core/RuleEvaluatorTest.java @@ -163,7 +163,7 @@ public Object[][] ruleEvaluationDataProvider() throws Exception { public void testEvaluateRule(Rule rule, Map evaluationData, boolean expectedResult) throws RuleEvaluationException { - boolean result = ruleEvaluator.evaluate(rule, evaluationData); + boolean result = ruleEvaluator.evaluate(rule, evaluationData).isRuleSatisfied(); if (expectedResult) { assertTrue(result); } else { @@ -182,7 +182,7 @@ public void testEvaluateFieldValueNotFound() throws Exception { @Test public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { - RuleEvaluationResult result = ruleEvaluator.evaluateResult( + RuleEvaluationResult result = ruleEvaluator.evaluate( createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testApp", "client-credentials")); @@ -194,7 +194,7 @@ public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { @Test public void testFailedFieldsEmptyWhenRulePasses() throws Exception { - RuleEvaluationResult result = ruleEvaluator.evaluateResult( + RuleEvaluationResult result = ruleEvaluator.evaluate( createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), createEvaluationData("testapp", "authorization_code")); From 77b993919933895e6f7feab6b5f82a6ec0389b86 Mon Sep 17 00:00:00 2001 From: kaviska Date: Tue, 14 Jul 2026 17:24:33 +0530 Subject: [PATCH 7/7] Remove unused Collections import in RuleEvaluator --- .../rule/evaluation/internal/service/impl/RuleEvaluator.java | 1 - 1 file changed, 1 deletion(-) diff --git a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java index 885013d40ec5..a982cbb5eb7f 100644 --- a/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java +++ b/components/rule-mgt/org.wso2.carbon.identity.rule.evaluation/src/main/java/org/wso2/carbon/identity/rule/evaluation/internal/service/impl/RuleEvaluator.java @@ -30,7 +30,6 @@ import org.wso2.carbon.identity.rule.management.api.model.Rule; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map;