Skip to content

Commit

Permalink
Merge pull request #114 from tkobayas/DROOLS-7635-error-incompatible-…
Browse files Browse the repository at this point in the history
…type-02

[DROOLS-7635] ansible-rulebook : Raise an error when a condition compares incompatible types
  • Loading branch information
tkobayas authored Oct 4, 2024
2 parents 7b3340d + ab41fe4 commit 2d3e3ec
Show file tree
Hide file tree
Showing 8 changed files with 623 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
@@ -0,0 +1,188 @@
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;
}

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;
}

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 (t == null
|| v == null
|| t instanceof Number && v instanceof Number
|| t.getClass() == v.getClass()) {
return predicate.test(t, v);
} else {
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
}
return false; // Different types are never equal
}
}

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
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package org.drools.ansible.rulebook.integration.api;

import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;

public class StringPrintStream extends PrintStream {

public static final String LINE_SEP = System.getProperty("line.separator");
PrintStream other;
List<String> stringList = new ArrayList<>();

public StringPrintStream(PrintStream ps) {
super(ps);
other = ps;
}

public void print(String s) {
other.print(s);
stringList.add(s);
}

public void println(String s) {
other.println(s);
stringList.add(s);
}

public void println(Object o) {
other.println(o);
stringList.add(o.toString());
}

public List<String> getStringList() {
return stringList;
}
}
Loading

0 comments on commit 2d3e3ec

Please sign in to comment.