Skip to content

Commit

Permalink
[DROOLS-7635] ansible-rulebook : Raise an error when a condition comp…
Browse files Browse the repository at this point in the history
…ares incompatible types

- WIP: This doesn't support alpha index. Some tests fail
  • Loading branch information
tkobayas committed Sep 26, 2024
1 parent 6b70da0 commit ab2d146
Show file tree
Hide file tree
Showing 6 changed files with 385 additions and 15 deletions.
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,179 @@
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.slf4j.Logger;
import org.slf4j.LoggerFactory;

import static org.drools.model.util.OperatorUtils.areEqual;
import static org.drools.model.util.OperatorUtils.compare;

public class RulebookConstraintOperator implements ConstraintOperator {

enum RulebookConstraintOperatorType {
EQUAL,
NOT_EQUAL,
GREATER_THAN,
GREATER_OR_EQUAL,
LESS_THAN,
LESS_OR_EQUAL,
UNKNOWN;
}

private static final Logger LOG = LoggerFactory.getLogger(RulebookConstraintOperator.class);

private RulebookConstraintOperatorType type;
private ConditionContext conditionContext;
private boolean typeCheckLogged = false;

public RulebookConstraintOperator(RulebookConstraintOperatorType type) {
this.type = type;
}

public void setConditionContext(RuleGenerationContext ruleContext, Map<?, ?> expression) {
this.conditionContext = new ConditionContext(ruleContext.getRuleSetName(), ruleContext.getRuleName(), expression.toString());
}

public RulebookConstraintOperator negate() {
switch (this.type) {
case EQUAL:
this.type = RulebookConstraintOperatorType.NOT_EQUAL;
return this;
case NOT_EQUAL:
this.type = RulebookConstraintOperatorType.EQUAL;
return this;
case GREATER_THAN:
this.type = RulebookConstraintOperatorType.LESS_OR_EQUAL;
return this;
case GREATER_OR_EQUAL:
this.type = RulebookConstraintOperatorType.LESS_THAN;
return this;
case LESS_OR_EQUAL:
this.type = RulebookConstraintOperatorType.GREATER_THAN;
return this;
case LESS_THAN:
this.type = RulebookConstraintOperatorType.GREATER_OR_EQUAL;
return this;
}
this.type = RulebookConstraintOperatorType.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) {
// TODO: rewrite the message to be python friendly
LOG.error("Cannot compare values of different types: {} and {}. RuleSet: {}. RuleName: {}. Condition: {}",
t.getClass(), 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 = RulebookConstraintOperatorType.LESS_THAN;
return this;
case GREATER_OR_EQUAL:
this.type = RulebookConstraintOperatorType.LESS_OR_EQUAL;
return this;
case LESS_THAN:
this.type = RulebookConstraintOperatorType.GREATER_THAN;
return this;
case LESS_OR_EQUAL:
this.type = RulebookConstraintOperatorType.GREATER_OR_EQUAL;
return this;
default:
return this;
}
}

public boolean isComparison() {
return isAscending() || isDescending();
}

public boolean isAscending() {
return this.type == RulebookConstraintOperatorType.GREATER_THAN || this.type == RulebookConstraintOperatorType.GREATER_OR_EQUAL;
}

public boolean isDescending() {
return this.type == RulebookConstraintOperatorType.LESS_THAN || this.type == RulebookConstraintOperatorType.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;
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package org.drools.ansible.rulebook.integration.api.domain.constraints;

import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
import org.drools.model.ConstraintOperator;
import org.drools.model.Index;

import java.util.Map;
import java.util.function.BiPredicate;

public interface RulebookOperator extends ConstraintOperator {
Expand All @@ -19,17 +20,43 @@ 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 OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.EQUAL));
}

static RulebookOperator newNotEqual() {
return new OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.NOT_EQUAL));
}

static RulebookOperator newGreaterThan() {
return new OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.GREATER_THAN));
}

static RulebookOperator newGreaterOrEqual() {
return new OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.GREATER_OR_EQUAL));
}

static RulebookOperator newLessThan() {
return new OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.LESS_THAN));
}

static RulebookOperator newLessOrEqual() {
return new OperatorWrapper(new RulebookConstraintOperator(RulebookConstraintOperator.RulebookConstraintOperatorType.LESS_OR_EQUAL));
}

default void setConditionContext(RuleGenerationContext ruleContext, Map<?, ?> expression) {
if (this instanceof OperatorWrapper operatorWrapper) {
operatorWrapper.setConditionContext(ruleContext, expression);
} else {
// do nothing
return;
}
}

class OperatorWrapper implements RulebookOperator {
private final Index.ConstraintType delegate;
private final RulebookConstraintOperator delegate;

public OperatorWrapper(Index.ConstraintType delegate) {
public OperatorWrapper(RulebookConstraintOperator delegate) {
this.delegate = delegate;
}

Expand All @@ -52,5 +79,9 @@ public RulebookOperator inverse() {
public ConstraintOperator asConstraintOperator() {
return delegate;
}

public void setConditionContext(RuleGenerationContext ruleContext, Map<?, ?> expression) {
delegate.setConditionContext(ruleContext, expression);
}
}
}
Loading

0 comments on commit ab2d146

Please sign in to comment.