[1.0.x][DROOLS-7635] ansible-rulebook : Raise an error when a condition compares incompatible types

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/LogUtil.java

@@ -0,0 +1,27 @@
+package org.drools.ansible.rulebook.integration.api;
+
+import java.util.Map;
+
+public class LogUtil {
+
+    private LogUtil() {
+        // utility class
+    }
+
+    // convert java class to python class
+    private static Map<Class<?>, String> convertMap = Map.of(
+            java.lang.Integer.class, "int",
+            java.lang.Boolean.class, "bool",
+            java.lang.String.class, "str",
+            java.lang.Double.class, "float",
+            java.util.List.class, "list",
+            java.util.ArrayList.class, "list",
+            java.util.Map.class, "dict",
+            java.util.LinkedHashMap.class, "dict",
+            java.util.HashMap.class, "dict"
+    );
+
+    public static String convertJavaClassToPythonClass(Class<?> javaClass) {
+        return convertMap.getOrDefault(javaClass, javaClass.getSimpleName());
+    }
+}

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/RuleGenerationContext.java

@@ -39,6 +39,8 @@ public class RuleGenerationContext {
 
     private final StackedContext<String, PrototypeDSL.PrototypePatternDef> patterns = new StackedContext<>();
 
+    private String ruleSetName;
+
     private String ruleName;
 
     private int bindingsCounter = 0;
@@ -209,11 +211,20 @@ public void setRuleName(String ruleName) {
         this.ruleName = ruleName;
     }
 
+    public String getRuleSetName() {
+        return ruleSetName;
+    }
+
+    public void setRuleSetName(String ruleSetName) {
+        this.ruleSetName = ruleSetName;
+    }
+
     List<Rule> toExecModelRules(RulesSet rulesSet, org.drools.ansible.rulebook.integration.api.domain.Rule ansibleRule, RulesExecutionController rulesExecutionController, AtomicInteger ruleCounter) {
     	updateContextFromRule(ansibleRule);
 	    if (getRuleName() == null) {
 	        setRuleName("r_" + ruleCounter.getAndIncrement());
 	    }
+        setRuleSetName(rulesSet.getName());
 
 	    List<org.drools.model.Rule> rules = createRules(rulesExecutionController);
 	    if (hasTemporalConstraint(ansibleRule)) {

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/MapCondition.java

@@ -218,6 +218,7 @@ private ParsedCondition parseSingle(RuleGenerationContext ruleContext, Map.Entry
 
     private ParsedCondition parseExpression(RuleGenerationContext ruleContext, String expressionName, Map<?, ?> expression) {
         RulebookOperator operator = decodeOperation(ruleContext, expressionName);
+        operator.setConditionContext(ruleContext, expression);
 
         if (operator instanceof ConditionFactory) {
             if (expression.get("lhs") != null) {
@@ -261,17 +262,17 @@ private static void throwExceptionIfCannotInverse(RulebookOperator operator, Rul
     private static RulebookOperator decodeOperation(RuleGenerationContext ruleContext, String expressionName) {
         switch (expressionName) {
             case "EqualsExpression":
-                return RulebookOperator.EQUAL;
+                return RulebookOperator.newEqual();
             case "NotEqualsExpression":
-                return RulebookOperator.NOT_EQUAL;
+                return RulebookOperator.newNotEqual();
             case "GreaterThanExpression":
-                return RulebookOperator.GREATER_THAN;
+                return RulebookOperator.newGreaterThan();
             case "GreaterThanOrEqualToExpression":
-                return RulebookOperator.GREATER_OR_EQUAL;
+                return RulebookOperator.newGreaterOrEqual();
             case "LessThanExpression":
-                return RulebookOperator.LESS_THAN;
+                return RulebookOperator.newLessThan();
             case "LessThanOrEqualToExpression":
-                return RulebookOperator.LESS_OR_EQUAL;
+                return RulebookOperator.newLessOrEqual();
             case ExistsField.EXPRESSION_NAME:
                 return ExistsField.INSTANCE;
             case ExistsField.NEGATED_EXPRESSION_NAME:

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NegationOperator.java

@@ -4,6 +4,8 @@
 
 import org.drools.model.ConstraintOperator;
 
+import static org.drools.ansible.rulebook.integration.api.domain.constraints.RulebookConstraintOperator.isCompatibleType;
+
 public class NegationOperator implements ConstraintOperator {
 
     private final ConstraintOperator toBeNegated;
@@ -14,6 +16,16 @@ public NegationOperator(ConstraintOperator toBeNegated) {
 
     @Override
     public <T, V> BiPredicate<T, V> asPredicate() {
+        if (toBeNegated instanceof RulebookConstraintOperator rulebookConstraintOperator) {
+            return (t, v) -> {
+                // if not compatible type, return false. Use the operator to log the type check error
+                if (!isCompatibleType(t, v)) {
+                    rulebookConstraintOperator.logTypeCheck(t, v);
+                    return false;
+                }
+                return !toBeNegated.asPredicate().test(t, v);
+            };
+        }
         return (t, v) -> !toBeNegated.asPredicate().test(t, v);
     }
 

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/RulebookConstraintOperator.java

@@ -0,0 +1,202 @@
+package org.drools.ansible.rulebook.integration.api.domain.constraints;
+
+import java.util.Map;
+import java.util.function.BiPredicate;
+
+import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
+import org.drools.model.Index;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.drools.ansible.rulebook.integration.api.LogUtil.convertJavaClassToPythonClass;
+import static org.drools.model.util.OperatorUtils.areEqual;
+import static org.drools.model.util.OperatorUtils.compare;
+
+public class RulebookConstraintOperator implements RulebookOperator {
+
+    private static final Logger LOG = LoggerFactory.getLogger(RulebookConstraintOperator.class);
+
+    private Index.ConstraintType type;
+    private ConditionContext conditionContext;
+    private boolean typeCheckLogged = false;
+
+    public RulebookConstraintOperator(Index.ConstraintType type) {
+        this.type = type;
+    }
+
+    @Override
+    public void setConditionContext(RuleGenerationContext ruleContext, Map<?, ?> expression) {
+        this.conditionContext = new ConditionContext(ruleContext.getRuleSetName(), ruleContext.getRuleName(), expression.toString());
+    }
+
+    @Override
+    public boolean hasIndex() {
+        return true;
+    }
+
+    @Override
+    public Index.ConstraintType getIndexType() {
+        return type;
+    }
+
+    public RulebookConstraintOperator negate() {
+        switch (this.type) {
+            case FORALL_SELF_JOIN:
+            case EQUAL:
+                this.type = Index.ConstraintType.NOT_EQUAL;
+                return this;
+            case NOT_EQUAL:
+                this.type = Index.ConstraintType.EQUAL;
+                return this;
+            case GREATER_THAN:
+                this.type = Index.ConstraintType.LESS_OR_EQUAL;
+                return this;
+            case GREATER_OR_EQUAL:
+                this.type = Index.ConstraintType.LESS_THAN;
+                return this;
+            case LESS_OR_EQUAL:
+                this.type = Index.ConstraintType.GREATER_THAN;
+                return this;
+            case LESS_THAN:
+                this.type = Index.ConstraintType.GREATER_OR_EQUAL;
+                return this;
+        }
+        this.type = Index.ConstraintType.UNKNOWN;
+        return this;
+    }
+
+    @Override
+    public boolean canInverse() {
+        switch (this.type) {
+            case EQUAL:
+            case NOT_EQUAL:
+            case GREATER_THAN:
+            case GREATER_OR_EQUAL:
+            case LESS_THAN:
+            case LESS_OR_EQUAL:
+                return true;
+            default:
+                return false;
+        }
+    }
+
+    @Override
+    public <T, V> BiPredicate<T, V> asPredicate() {
+        final BiPredicate<T, V> predicate;
+        switch (this.type) {
+            case EQUAL:
+                predicate = (t, v) -> areEqual(t, v);
+                break;
+            case NOT_EQUAL:
+                predicate = (t, v) -> !areEqual(t, v);
+                break;
+            case GREATER_THAN:
+                predicate = (t,v) -> t != null && compare(t, v) > 0;
+                break;
+            case GREATER_OR_EQUAL:
+                predicate = (t,v) -> t != null && compare(t, v) >= 0;
+                break;
+            case LESS_THAN:
+                predicate = (t,v) -> t != null && compare(t, v) < 0;
+                break;
+            case LESS_OR_EQUAL:
+                predicate = (t,v) -> t != null && compare(t, v) <= 0;
+                break;
+            default:
+                throw new UnsupportedOperationException("Cannot convert " + this + " into a predicate");
+        }
+        return (t, v) -> predicateWithTypeCheck(t, v, predicate);
+    }
+
+    private <T, V> boolean predicateWithTypeCheck(T t, V v, BiPredicate<T, V> predicate) {
+        if (isCompatibleType(t, v)) {
+            return predicate.test(t, v);
+        } else {
+            logTypeCheck(t, v);
+            return false; // Different types evaluation always return false even if the operator is NOT_EQUAL
+        }
+    }
+
+    /*
+     * Log a type check error once per constraint
+     */
+    <T, V> void logTypeCheck(T t, V v) {
+        if (!typeCheckLogged) {
+            LOG.error("Cannot compare values of different types: {} and {}. RuleSet: {}. RuleName: {}. Condition: {}",
+                      convertJavaClassToPythonClass(t.getClass()),
+                      convertJavaClassToPythonClass(v.getClass()),
+                      conditionContext.getRuleSet(), conditionContext.getRuleName(), conditionContext.getConditionExpression());
+            typeCheckLogged = true; // Log only once per constraint
+        }
+    }
+
+    public static boolean isCompatibleType(Object t, Object v) {
+        return t == null
+                || v == null
+                || t instanceof Number && v instanceof Number
+                || t.getClass() == v.getClass();
+    }
+
+    @Override
+    public RulebookConstraintOperator inverse() {
+        switch (this.type) {
+            case GREATER_THAN:
+                this.type = Index.ConstraintType.LESS_THAN;
+                return this;
+            case GREATER_OR_EQUAL:
+                this.type =  Index.ConstraintType.LESS_OR_EQUAL;
+                return this;
+            case LESS_THAN:
+                this.type = Index.ConstraintType.GREATER_THAN;
+                return this;
+            case LESS_OR_EQUAL:
+                this.type = Index.ConstraintType.GREATER_OR_EQUAL;
+                return this;
+            default:
+                return this;
+        }
+    }
+
+    public boolean isComparison() {
+        return isAscending() || isDescending();
+    }
+
+    public boolean isAscending() {
+        return this.type == Index.ConstraintType.GREATER_THAN || this.type == Index.ConstraintType.GREATER_OR_EQUAL;
+    }
+
+    public boolean isDescending() {
+        return this.type == Index.ConstraintType.LESS_THAN || this.type == Index.ConstraintType.LESS_OR_EQUAL;
+    }
+
+    private class ConditionContext {
+
+        private String ruleSet;
+        private String ruleName;
+        private String conditionExpression;
+
+        public ConditionContext(String ruleSet, String ruleName, String conditionExpression) {
+            this.ruleSet = ruleSet;
+            this.ruleName = ruleName;
+            this.conditionExpression = conditionExpression;
+        }
+
+        public String getRuleSet() {
+            return ruleSet;
+        }
+
+        public String getRuleName() {
+            return ruleName;
+        }
+
+        public String getConditionExpression() {
+            return conditionExpression;
+        }
+    }
+
+    @Override
+    public String toString() {
+        // works for node sharing
+        return type.toString();
+    }
+}

drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/RulebookOperator.java

@@ -1,10 +1,12 @@
 package org.drools.ansible.rulebook.integration.api.domain.constraints;
 
+import java.util.Map;
+import java.util.function.BiPredicate;
+
+import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
 import org.drools.model.ConstraintOperator;
 import org.drools.model.Index;
 
-import java.util.function.BiPredicate;
-
 public interface RulebookOperator extends ConstraintOperator {
 
     default boolean canInverse() {
@@ -19,38 +21,35 @@ default ConstraintOperator asConstraintOperator() {
         return this;
     }
 
-    RulebookOperator EQUAL = new OperatorWrapper(Index.ConstraintType.EQUAL);
-    RulebookOperator NOT_EQUAL = new OperatorWrapper(Index.ConstraintType.NOT_EQUAL);
-    RulebookOperator GREATER_THAN = new OperatorWrapper(Index.ConstraintType.GREATER_THAN);
-    RulebookOperator GREATER_OR_EQUAL = new OperatorWrapper(Index.ConstraintType.GREATER_OR_EQUAL);
-    RulebookOperator LESS_THAN = new OperatorWrapper(Index.ConstraintType.LESS_THAN);
-    RulebookOperator LESS_OR_EQUAL = new OperatorWrapper(Index.ConstraintType.LESS_OR_EQUAL);
+    static RulebookOperator newEqual() {
+        return new RulebookConstraintOperator(Index.ConstraintType.EQUAL);
+    }
 
-    class OperatorWrapper implements RulebookOperator {
-        private final Index.ConstraintType delegate;
+    static RulebookOperator newNotEqual() {
+        return new RulebookConstraintOperator(Index.ConstraintType.NOT_EQUAL);
+    }
 
-        public OperatorWrapper(Index.ConstraintType delegate) {
-            this.delegate = delegate;
-        }
+    static RulebookOperator newGreaterThan() {
+        return new RulebookConstraintOperator(Index.ConstraintType.GREATER_THAN);
+    }
 
-        @Override
-        public <T, V> BiPredicate<T, V> asPredicate() {
-            return delegate.asPredicate();
-        }
+    static RulebookOperator newGreaterOrEqual() {
+        return new RulebookConstraintOperator(Index.ConstraintType.GREATER_OR_EQUAL);
+    }
 
-        @Override
-        public boolean canInverse() {
-            return delegate.canInverse();
-        }
+    static RulebookOperator newLessThan() {
+        return new RulebookConstraintOperator(Index.ConstraintType.LESS_THAN);
+    }
 
-        @Override
-        public RulebookOperator inverse() {
-            return new OperatorWrapper(delegate.inverse());
-        }
+    static RulebookOperator newLessOrEqual() {
+        return new RulebookConstraintOperator(Index.ConstraintType.LESS_OR_EQUAL);
+    }
 
-        @Override
-        public ConstraintOperator asConstraintOperator() {
-            return delegate;
+    default void setConditionContext(RuleGenerationContext ruleContext, Map<?, ?> expression) {
+        if (this instanceof RulebookConstraintOperator rulebookConstraintOperator) {
+            rulebookConstraintOperator.setConditionContext(ruleContext, expression);
+        } else {
+            // do nothing
         }
     }
 }