Skip to content

Commit

Permalink
[1.0.x][DROOLS-7635] ansible-rulebook : Raise an error when a conditi…
Browse files Browse the repository at this point in the history
…on compares incompatible types (#120)

* Merge pull request #114 from tkobayas/DROOLS-7635-error-incompatible-type-02

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

* [DROOLS-7635] ansible-rulebook : Raise an error when a condition comp… (#117)

* [DROOLS-7635] ansible-rulebook : Raise an error when a condition compares incompatible types
- alpha index test, beta index test

beta index test fix

add negate test and any test

* refactor

* minor comment fix
  • Loading branch information
tkobayas authored Oct 18, 2024
1 parent ad8e327 commit 25ac2c7
Show file tree
Hide file tree
Showing 8 changed files with 939 additions and 34 deletions.
Original file line number Diff line number Diff line change
@@ -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());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
@@ -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() {
Expand All @@ -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
}
}
}
Loading

0 comments on commit 25ac2c7

Please sign in to comment.