From 6b70da010b474dbf8fa4969d8c6332ad11e2d88f Mon Sep 17 00:00:00 2001 From: Mario Fusco Date: Wed, 11 Sep 2024 14:40:07 +0200 Subject: [PATCH] Fix evaluation of is not defined expression (#108) --- .../api/domain/conditions/AstCondition.java | 30 +++++ .../conditions/BetaExpressionCondition.java | 29 ---- .../api/domain/conditions/Condition.java | 1 + .../conditions/ExpressionCondition.java | 32 ----- .../api/domain/conditions/MapCondition.java | 42 +++++- .../domain/conditions/SimpleCondition.java | 5 + .../constraints/NegatedExistsField.java | 126 ++++++++++++++++++ .../domain/constraints/NotExistsField.java | 21 --- .../integration/api/ProcessEventTest.java | 105 ++++++++++++++- 9 files changed, 305 insertions(+), 86 deletions(-) delete mode 100644 drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/BetaExpressionCondition.java delete mode 100644 drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/ExpressionCondition.java create mode 100644 drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NegatedExistsField.java delete mode 100644 drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NotExistsField.java diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/AstCondition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/AstCondition.java index c73b4dd7..d990a970 100644 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/AstCondition.java +++ b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/AstCondition.java @@ -36,6 +36,11 @@ public AnyCondition any() { return anyCondition; } + @Override + public boolean isSingleCondition() { + return rootCondition.isSingleCondition(); + } + @Override public ViewItem toPattern(RuleGenerationContext ruleContext) { return rootCondition.toPattern(ruleContext); @@ -93,6 +98,11 @@ public AllCondition(RuleGenerationContext ruleContext) { protected org.drools.model.Condition.Type getConditionType() { return org.drools.model.Condition.Type.AND; } + + @Override + public boolean isSingleCondition() { + return false; + } } public static class AnyCondition extends MultipleConditions { @@ -115,6 +125,11 @@ protected void beforeBinding() { protected void afterBinding() { ruleContext.popContext(); } + + @Override + public boolean isSingleCondition() { + return false; + } } public abstract static class PatternCondition implements Condition { @@ -187,6 +202,11 @@ public PatternCondition negate(RuleGenerationContext ruleContext) { .withLhs(lhs.negate(ruleContext)) .withRhs(rhs.negate(ruleContext)); } + + @Override + public boolean isSingleCondition() { + return false; + } } public static class OrCondition extends CombinedPatternCondition { @@ -221,6 +241,11 @@ public PatternCondition negate(RuleGenerationContext ruleContext) { .withLhs(lhs.negate(ruleContext)) .withRhs(rhs.negate(ruleContext)); } + + @Override + public boolean isSingleCondition() { + return false; + } } public static class SingleCondition

extends PatternCondition { @@ -283,5 +308,10 @@ public SingleCondition

addSingleCondition(PrototypeExpression left, Index.Con public ParsedCondition getParsedCondition() { return parsedCondition; } + + @Override + public boolean isSingleCondition() { + return true; + } } } \ No newline at end of file diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/BetaExpressionCondition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/BetaExpressionCondition.java deleted file mode 100644 index 7af61c93..00000000 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/BetaExpressionCondition.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.drools.ansible.rulebook.integration.api.domain.conditions; - -import org.drools.model.ConstraintOperator; -import org.drools.model.prototype.PrototypeExpression; - -public class BetaExpressionCondition extends ExpressionCondition { - - private final String otherBinding; - - public BetaExpressionCondition(String patternBinding, PrototypeExpression left, ConstraintOperator operator, String otherBinding, PrototypeExpression right) { - super(patternBinding, left, operator, right); - this.otherBinding = otherBinding; - } - - @Override - public String otherBinding() { - return otherBinding; - } - - @Override - public boolean beta() { - return true; - } - - @Override - public String toString() { - return left + " " + operator + " " + otherBinding + "." + right; - } -} diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/Condition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/Condition.java index 7002b1fc..3223d338 100644 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/Condition.java +++ b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/Condition.java @@ -5,4 +5,5 @@ public interface Condition { ViewItem toPattern(RuleGenerationContext ruleContext); + boolean isSingleCondition(); } diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/ExpressionCondition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/ExpressionCondition.java deleted file mode 100644 index 3acd139c..00000000 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/ExpressionCondition.java +++ /dev/null @@ -1,32 +0,0 @@ -package org.drools.ansible.rulebook.integration.api.domain.conditions; - -import org.drools.ansible.rulebook.integration.api.rulesmodel.ParsedCondition; -import org.drools.model.ConstraintOperator; -import org.drools.model.prototype.PrototypeExpression; - -public class ExpressionCondition extends SimpleCondition { - protected final PrototypeExpression left; - protected final ConstraintOperator operator; - protected final PrototypeExpression right; - - public ExpressionCondition(PrototypeExpression left, ConstraintOperator operator, PrototypeExpression right) { - this.left = left; - this.operator = operator; - this.right = right; - } - - public ExpressionCondition(String patternBinding, PrototypeExpression left, ConstraintOperator operator, PrototypeExpression right) { - this(left, operator, right); - setPatternBinding(patternBinding); - } - - @Override - public ParsedCondition parse() { - return new ParsedCondition(left, operator, right); - } - - @Override - public String toString() { - return left + " " + operator + " " + right; - } -} diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/MapCondition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/MapCondition.java index 38b5bde4..f65afbe4 100644 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/MapCondition.java +++ b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/MapCondition.java @@ -7,6 +7,7 @@ import org.drools.ansible.rulebook.integration.api.domain.constraints.ItemNotInListConstraint; import org.drools.ansible.rulebook.integration.api.domain.constraints.ListContainsConstraint; import org.drools.ansible.rulebook.integration.api.domain.constraints.ListNotContainsConstraint; +import org.drools.ansible.rulebook.integration.api.domain.constraints.NegatedExistsField; import org.drools.ansible.rulebook.integration.api.domain.constraints.RulebookOperator; import org.drools.ansible.rulebook.integration.api.domain.constraints.SearchMatchesConstraint; import org.drools.ansible.rulebook.integration.api.domain.constraints.SelectAttrConstraint; @@ -48,6 +49,39 @@ public void setMap(Map map) { this.map = map; } + @Override + public boolean isSingleCondition() { + return isSingleCondition(map); + } + + private boolean isSingleCondition(Map map) { + if (map.size() != 1) { + return false; + } + Object value = map.values().iterator().next(); + if (value instanceof Map) { + return isSingleCondition((Map) value); + } + if (value instanceof List) { + return isSingleCondition((List) value); + } + return true; + } + + private boolean isSingleCondition(List list) { + if (list.size() != 1) { + return false; + } + Object value = list.get(0); + if (value instanceof Map) { + return isSingleCondition((Map) value); + } + if (value instanceof List) { + return isSingleCondition((List) value); + } + return true; + } + private String getPatternBinding(RuleGenerationContext ruleContext) { if (patternBinding == null) { patternBinding = ruleContext.generateBinding(); @@ -183,7 +217,7 @@ private ParsedCondition parseSingle(RuleGenerationContext ruleContext, Map.Entry } private ParsedCondition parseExpression(RuleGenerationContext ruleContext, String expressionName, Map expression) { - RulebookOperator operator = decodeOperation(expressionName); + RulebookOperator operator = decodeOperation(ruleContext, expressionName); if (operator instanceof ConditionFactory) { if (expression.get("lhs") != null) { @@ -224,7 +258,7 @@ private static void throwExceptionIfCannotInverse(RulebookOperator operator, Rul } } - private static RulebookOperator decodeOperation(String expressionName) { + private static RulebookOperator decodeOperation(RuleGenerationContext ruleContext, String expressionName) { switch (expressionName) { case "EqualsExpression": return RulebookOperator.EQUAL; @@ -239,8 +273,10 @@ private static RulebookOperator decodeOperation(String expressionName) { case "LessThanOrEqualToExpression": return RulebookOperator.LESS_OR_EQUAL; case ExistsField.EXPRESSION_NAME: - case ExistsField.NEGATED_EXPRESSION_NAME: return ExistsField.INSTANCE; + case ExistsField.NEGATED_EXPRESSION_NAME: + // IsNotDefinedExpression behaves as an existential not only when used alone + return ruleContext.getCondition().isSingleCondition() ? ExistsField.INSTANCE : NegatedExistsField.INSTANCE; case ListContainsConstraint.EXPRESSION_NAME: return ListContainsConstraint.INSTANCE; case ListNotContainsConstraint.EXPRESSION_NAME: diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/SimpleCondition.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/SimpleCondition.java index 40913700..f16274d4 100644 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/SimpleCondition.java +++ b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/conditions/SimpleCondition.java @@ -88,6 +88,11 @@ public Type getType() { return Type.SINGLE; } + @Override + public boolean isSingleCondition() { + return getType() == Type.SINGLE; + } + public String otherBinding() { throw new UnsupportedOperationException(); } diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NegatedExistsField.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NegatedExistsField.java new file mode 100644 index 00000000..62d522c4 --- /dev/null +++ b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NegatedExistsField.java @@ -0,0 +1,126 @@ +package org.drools.ansible.rulebook.integration.api.domain.constraints; + +import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext; +import org.drools.ansible.rulebook.integration.api.domain.conditions.ConditionExpression; +import org.drools.ansible.rulebook.integration.api.rulesmodel.ParsedCondition; +import org.drools.model.functions.Function1; +import org.drools.model.prototype.PrototypeExpression; +import org.drools.model.prototype.PrototypeVariable; +import org.kie.api.prototype.Prototype; +import org.kie.api.prototype.PrototypeFactInstance; + +import java.util.Collection; +import java.util.Map; +import java.util.Optional; +import java.util.function.BiPredicate; + +import static org.drools.ansible.rulebook.integration.api.domain.conditions.ConditionExpression.map2Expr; +import static org.drools.model.prototype.PrototypeExpression.thisPrototype; + +public enum NegatedExistsField implements RulebookOperator, ConditionFactory { + + INSTANCE; + + public static final String EXPRESSION_NAME = "IsNotDefinedExpression"; + + private static final Object ADMITTED_UNDEFINED_VALUE = AdmittedUndefinedValue.INSTANCE; + + @Override + public BiPredicate asPredicate() { + return (t, v) -> v == ADMITTED_UNDEFINED_VALUE; + } + + @Override + public String toString() { + return "NEGATED_EXISTS_FIELD"; + } + + @Override + public ParsedCondition createParsedCondition(RuleGenerationContext ruleContext, String expressionName, Map expression) { + PrototypeExpression rightExpression = map2Expr(ruleContext, expression).getPrototypeExpression(); + return new ParsedCondition(thisPrototype(), this, new PrototypeExpressionWithAdmittedUndefined(rightExpression)); + } + + private static class PrototypeExpressionWithAdmittedUndefined implements PrototypeExpression { + private final PrototypeExpression delegate; + + private PrototypeExpressionWithAdmittedUndefined(PrototypeExpression delegate) { + this.delegate = delegate; + } + + @Override + public Function1 asFunction(Prototype prototype) { + return delegate.asFunction(prototype).andThen( result -> result == Prototype.UNDEFINED_VALUE ? ADMITTED_UNDEFINED_VALUE : result ); + } + + @Override + public Collection getImpactedFields() { + return delegate.getImpactedFields(); + } + + @Override + public Optional getIndexingKey() { + return delegate.getIndexingKey(); + } + + @Override + public PrototypeExpression andThen(PrototypeExpression other) { + return delegate.andThen(other); + } + + @Override + public boolean hasPrototypeVariable() { + return delegate.hasPrototypeVariable(); + } + + @Override + public Collection getPrototypeVariables() { + return delegate.getPrototypeVariables(); + } + + @Override + public PrototypeExpression composeWith(BinaryOperation.Operator op, PrototypeExpression right) { + return delegate.composeWith(op, right); + } + + @Override + public PrototypeExpression add(PrototypeExpression right) { + return delegate.add(right); + } + + @Override + public PrototypeExpression sub(PrototypeExpression right) { + return delegate.sub(right); + } + + @Override + public PrototypeExpression mul(PrototypeExpression right) { + return delegate.mul(right); + } + + @Override + public PrototypeExpression div(PrototypeExpression right) { + return delegate.div(right); + } + } + + public static class AdmittedUndefinedValue { + static final AdmittedUndefinedValue INSTANCE = new AdmittedUndefinedValue(); + static final UnsupportedOperationException HASHCODE_EXCEPTION = new UnsupportedOperationException(); + + public AdmittedUndefinedValue() { + } + + public int hashCode() { + throw HASHCODE_EXCEPTION; + } + + public boolean equals(Object obj) { + return false; + } + + public String toString() { + return "$AdmittedUndefinedValue$"; + } + } +} diff --git a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NotExistsField.java b/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NotExistsField.java deleted file mode 100644 index b6746275..00000000 --- a/drools-ansible-rulebook-integration-api/src/main/java/org/drools/ansible/rulebook/integration/api/domain/constraints/NotExistsField.java +++ /dev/null @@ -1,21 +0,0 @@ -package org.drools.ansible.rulebook.integration.api.domain.constraints; - -import org.drools.model.ConstraintOperator; - -import java.util.function.BiPredicate; - -@Deprecated() // TODO seems no longer in-use? -public enum NotExistsField implements ConstraintOperator { - - INSTANCE; - - @Override - public BiPredicate asPredicate() { - throw new UnsupportedOperationException("deprecated and should be no longer in use"); - } - - @Override - public String toString() { - return "NOT_EXISTS_FIELD"; - } -} diff --git a/drools-ansible-rulebook-integration-api/src/test/java/org/drools/ansible/rulebook/integration/api/ProcessEventTest.java b/drools-ansible-rulebook-integration-api/src/test/java/org/drools/ansible/rulebook/integration/api/ProcessEventTest.java index ca1e972b..2691afbf 100644 --- a/drools-ansible-rulebook-integration-api/src/test/java/org/drools/ansible/rulebook/integration/api/ProcessEventTest.java +++ b/drools-ansible-rulebook-integration-api/src/test/java/org/drools/ansible/rulebook/integration/api/ProcessEventTest.java @@ -189,4 +189,107 @@ public void isNotDefinedExpression() { rulesExecutor.dispose(); } -} + + public static final String JSON_IS_DEFINED_AND_IS_NOT_DEFINED = + """ + { + "rules": [ + { + "Rule": { + "name": "r1", + "condition": { + "AllCondition": [ + { + "AndExpression": { + "lhs": { + "IsDefinedExpression": { + "Event": "meta" + } + }, + "rhs": { + "IsNotDefinedExpression": { + "Event": "meta.headers.token" + } + } + } + } + ] + }, + "actions": [ + { + "Action": { + "action": "debug", + "action_args": {} + } + } + ], + "enabled": true + } + } + ] + } + """; + + @Test + public void isDefinedAndIsNotDefinedExpression() { + RulesExecutor rulesExecutor = RulesExecutorFactory.createFromJson(JSON_IS_DEFINED_AND_IS_NOT_DEFINED); + + List matchedRules = rulesExecutor.processEvents("{\"meta\":{\"headers\":{\"token\":123}}}").join(); + assertEquals(0, matchedRules.size()); + + matchedRules = rulesExecutor.processEvents("{\"meta\":{\"headers\":{\"age\":23}}}").join(); + assertEquals(1, matchedRules.size()); + assertEquals("r1", matchedRules.get(0).getRule().getName()); + + rulesExecutor.dispose(); + } + + public static final String JSON_IS_DEFINED_IS_NOT_DEFINED_IN_ALL = + """ + { + "rules": [ + { + "Rule": { + "name": "r1", + "condition": { + "AllCondition": [ + { + "IsDefinedExpression": { + "Event": "meta" + } + }, + { + "IsNotDefinedExpression": { + "Event": "meta.headers.token" + } + } + ] + }, + "actions": [ + { + "Action": { + "action": "debug", + "action_args": {} + } + } + ], + "enabled": true + } + } + ] + } + """; + + @Test + public void isDefinedIsNotDefinedInAllExpression() { + RulesExecutor rulesExecutor = RulesExecutorFactory.createFromJson(JSON_IS_DEFINED_IS_NOT_DEFINED_IN_ALL); + + List matchedRules = rulesExecutor.processEvents("{\"meta\":{\"headers\":{\"token\":123}}}").join(); + assertEquals(0, matchedRules.size()); + + matchedRules = rulesExecutor.processEvents("{\"meta\":{\"headers\":{\"age\":23}}}").join(); + assertEquals(1, matchedRules.size()); + assertEquals("r1", matchedRules.get(0).getRule().getName()); + + rulesExecutor.dispose(); + }}