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..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 @@ -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,27 @@ public class RuleEvaluationResult { private final String ruleId; 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, null); + } + + public RuleEvaluationResult(String ruleId, boolean ruleSatisfied, List failedFields) { + this.ruleId = ruleId; this.ruleSatisfied = ruleSatisfied; + // Keep as-is: null means the failed fields were not set, which is different from an empty list. + this.failedFields = failedFields; } public String getRuleId() { @@ -42,4 +61,15 @@ public boolean isRuleSatisfied() { return ruleSatisfied; } + + /** + * 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 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 854f1bc36c60..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 @@ -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,12 @@ 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(rule, evaluationData); + if (LOG.isDebugEnabled()) { + LOG.debug("Evaluated rule: " + rule.getId() + " to: " + result.isRuleSatisfied() + "."); + } - return new RuleEvaluationResult(ruleId, evaluationStatus); + 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 3aa2b4b03bdf..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 @@ -23,11 +23,13 @@ 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; import org.wso2.carbon.identity.rule.management.api.model.Rule; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -58,35 +60,44 @@ public RuleEvaluator(OperatorRegistry operatorRegistry) { } /** - * Evaluate a given rule. + * Evaluate a given rule and return the result, including the fields that failed evaluation. * * @param rule Rule to evaluate. * @param evaluationData Evaluation data. - * @return Evaluation result. + * @return Rule evaluation result with the satisfied status and the failed fields. * @throws RuleEvaluationException If an error occurs while evaluating the rule. */ - public boolean evaluate(Rule rule, Map evaluationData) throws RuleEvaluationException { + public RuleEvaluationResult evaluate(Rule rule, Map evaluationData) + throws RuleEvaluationException { + List failedFields = new ArrayList<>(); ORCombinedRule orRule = (ORCombinedRule) rule; - return evaluateORCombinedRule(orRule, evaluationData); + boolean ruleSatisfied = evaluateORCombinedRule(orRule, evaluationData, failedFields); + return new RuleEvaluationResult(rule.getId(), 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()) { - 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 } } 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..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 @@ -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; @@ -60,6 +61,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; @@ -161,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 { @@ -177,6 +179,29 @@ public void testEvaluateFieldValueNotFound() throws Exception { Collections.emptyMap()); } + @Test + public void testFailedFieldsPopulatedWhenRuleFails() throws Exception { + + RuleEvaluationResult result = ruleEvaluator.evaluate( + createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), + createEvaluationData("testApp", "client-credentials")); + + assertFalse(result.isRuleSatisfied()); + // The rule fails on the first failing expression (application), so it is reported as a failed field. + assertEquals(result.getFailedFields(), Collections.singletonList("application")); + } + + @Test + public void testFailedFieldsEmptyWhenRulePasses() throws Exception { + + RuleEvaluationResult result = ruleEvaluator.evaluate( + createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes(), + createEvaluationData("testapp", "authorization_code")); + + assertTrue(result.isRuleSatisfied()); + assertTrue(result.getFailedFields().isEmpty()); + } + private Rule createRuleWithTwoANDExpressionsUsingReferenceAndStringValueTypes() throws Exception { RuleBuilder ruleBuilder = RuleBuilder.create(FlowType.PRE_ISSUE_ACCESS_TOKEN, "tenant1");