Skip to content

Commit

Permalink
DROOLS-7475 Proposed feature for ['key'] accessor (#56)
Browse files Browse the repository at this point in the history
* DROOLS-7475 Proposed feature for ['key'] accessor

nesting the protoextractor into the maven build

* WIP: broken but progressing

Results :

Failed tests:   testWithSelectAttr(org.drools.ansible.rulebook.integration.api.NullTest): expected:<1> but was:<0>
  test(org.drools.ansible.rulebook.integration.api.NullTest): expected:<1> but was:<0>
  testRepeatedOnceWithin(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithOr(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithAnd(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testSelectOnNull(org.drools.ansible.rulebook.integration.api.SelectTest): expected:<1> but was:<0>

Tests in error:
  testOnceWithinInRule(org.drools.ansible.rulebook.integration.api.OnceWithinTest): Cannot invoke "java.util.Map.get(Object)" because "map" is null
  testOnceWithinInCondition(org.drools.ansible.rulebook.integration.api.OnceWithinTest): Cannot invoke "java.util.Map.get(Object)" because "map" is null
  testOnceAfterWithOr(org.drools.ansible.rulebook.integration.api.OnceAfterTest): Cannot invoke "Object.toString()" because the return value of "org.drools.base.facttemplates.Fact.get(String)" is null

Tests run: 138, Failures: 6, Errors: 3, Skipped: 0

* WIP: broken but null != UNDEF

Results :

Failed tests:   testRepeatedOnceWithin(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithOr(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithAnd(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>

Tests in error:
  testOnceWithinInRule(org.drools.ansible.rulebook.integration.api.OnceWithinTest): Cannot invoke "java.util.Map.get(Object)" because "map" is null
  testOnceWithinInCondition(org.drools.ansible.rulebook.integration.api.OnceWithinTest): Cannot invoke "java.util.Map.get(Object)" because "map" is null
  testOnceAfterWithOr(org.drools.ansible.rulebook.integration.api.OnceAfterTest): Cannot invoke "Object.toString()" because the return value of "org.drools.base.facttemplates.Fact.get(String)" is null

Tests run: 138, Failures: 3, Errors: 3, Skipped: 0

* WIP: broken but progressing

Failed tests:   testRepeatedOnceWithin(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinInRule(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithOr(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinWithAnd(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceWithinInCondition(org.drools.ansible.rulebook.integration.api.OnceWithinTest): expected:<0> but was:<1>
  testOnceAfterWithOr(org.drools.ansible.rulebook.integration.api.OnceAfterTest): expected:<2> but was:<1>

Tests run: 138, Failures: 6, Errors: 0, Skipped: 0

* ALL tests passing

* small refactor

* cleanup RulesModelUtil

* refactor, small fix for array access, tests

* fix grammar typo

* refactor ExistsField as discussed with Mario

* implement code review feedback

* implement code review feedback

* code review feedback impl about ORIGINAL_MAP_FIELD

* BREAKING to investigate indexing issue

* revert the disabled commit

* implement code review feedback

* code review feedback
  • Loading branch information
tarilabs authored Jun 22, 2023
1 parent d4aa1ab commit 28b41e5
Show file tree
Hide file tree
Showing 51 changed files with 1,253 additions and 78 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@ dependency-reduced-pom.xml

#CI
!.ci

.antlr
5 changes: 5 additions & 0 deletions drools-ansible-rulebook-integration-api/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
<name>Drools :: Ansible Rulebook Integration :: API</name>

<dependencies>
<dependency>
<groupId>org.drools</groupId>
<artifactId>drools-ansible-rulebook-integration-protoextractor</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-simple</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.drools.ansible.rulebook.integration.api.rulesengine.RulesExecutorSession;
import org.drools.ansible.rulebook.integration.api.rulesengine.SessionStats;
import org.drools.base.facttemplates.Fact;
import org.kie.api.runtime.KieSession;
import org.kie.api.runtime.rule.Match;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -95,4 +96,8 @@ public String getAllFactsAsJson() {
public CompletableFuture<List<Match>> advanceTime(long amount, TimeUnit unit ) {
return rulesEvaluator.advanceTime(amount, unit );
}

public KieSession asKieSession() {
return rulesEvaluator.asKieSession();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import java.util.Map;

import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpression;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpressionUtils;
import org.drools.model.PrototypeDSL;
import org.drools.model.PrototypeExpression;
import org.drools.model.PrototypeVariable;
Expand Down Expand Up @@ -37,8 +39,8 @@ public ConditionExpression(PrototypeExpression prototypeExpression, boolean fiel

public ConditionExpression composeWith(PrototypeExpression.BinaryOperation.Operator decodeBinaryOperator, ConditionExpression rhs) {
if (isBeta() && rhs.isBeta() && !prototypeName.equals(rhs.getPrototypeName())) {
PrototypeExpression composed = prototypeField(betaVariable, getFieldName())
.composeWith(decodeBinaryOperator, prototypeField(rhs.getBetaVariable(), rhs.getFieldName()));
PrototypeExpression composed = ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(betaVariable, getFieldName())
.composeWith(decodeBinaryOperator, ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(rhs.getBetaVariable(), rhs.getFieldName()));
return new ConditionExpression(composed);
}

Expand All @@ -54,7 +56,7 @@ public boolean isBeta() {
}

public String getFieldName() {
return ((PrototypeExpression.PrototypeFieldValue) prototypeExpression).getFieldName();
return prototypeExpression.getIndexingKey().orElseThrow(IllegalStateException::new);
}

public PrototypeExpression getPrototypeExpression() {
Expand Down Expand Up @@ -142,7 +144,7 @@ private static ConditionExpression createFieldExpression(RuleGenerationContext r
betaVariable = (PrototypeVariable) boundPattern.getFirstVariable();
}
}
return new ConditionExpression(fieldName2PrototypeExpression(fieldName), true, prototypeName, betaVariable);
return new ConditionExpression(ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(fieldName), true, prototypeName, betaVariable);
}

private static PrototypeExpression.BinaryOperation.Operator decodeBinaryOperator(String operator) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,9 @@
import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
import org.drools.ansible.rulebook.integration.api.rulesmodel.ParsedCondition;
import org.drools.model.ConstraintOperator;
import org.drools.model.PrototypeFact;
import org.drools.model.Prototype;

import static org.drools.ansible.rulebook.integration.api.domain.conditions.ConditionExpression.map2Expr;
import static org.drools.model.PrototypeExpression.fixedValue;
import static org.drools.model.PrototypeExpression.thisPrototype;

public enum ExistsField implements ConstraintOperator, ConditionFactory {
Expand All @@ -21,7 +20,7 @@ public enum ExistsField implements ConstraintOperator, ConditionFactory {

@Override
public <T, V> BiPredicate<T, V> asPredicate() {
return (t,v) -> ((PrototypeFact) t).has((String) v);
return (t, v) -> v != Prototype.UNDEFINED_VALUE; // actually, always true from caller: https://github.com/kiegroup/drools/blob/9de8d0b54b364bda1b1d76d81923c8bfc060c2f8/drools-model/drools-canonical-model/src/main/java/org/drools/model/PrototypeDSL.java#L273
}

@Override
Expand All @@ -31,6 +30,6 @@ public String toString() {

@Override
public ParsedCondition createParsedCondition(RuleGenerationContext ruleContext, String expressionName, Map<?, ?> expression) {
return new ParsedCondition(thisPrototype(), this, fixedValue(map2Expr(ruleContext, expression).getFieldName())).withNotPattern(expressionName.equals(NEGATED_EXPRESSION_NAME));
return new ParsedCondition(thisPrototype(), this, map2Expr(ruleContext, expression).getPrototypeExpression()).withNotPattern(expressionName.equals(NEGATED_EXPRESSION_NAME));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
import org.drools.model.ConstraintOperator;
import org.drools.model.PrototypeFact;

@Deprecated() // TODO seems no longer in-use?
public enum NotExistsField implements ConstraintOperator {

INSTANCE;

@Override
public <T, V> BiPredicate<T, V> asPredicate() {
return (t,v) -> !((PrototypeFact) t).has((String) v);
throw new UnsupportedOperationException("deprecated and should be no longer in use");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import org.drools.ansible.rulebook.integration.api.domain.conditions.ConditionExpression;
import org.drools.ansible.rulebook.integration.api.rulesmodel.BetaParsedCondition;
import org.drools.ansible.rulebook.integration.api.rulesmodel.ParsedCondition;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpressionUtils;
import org.drools.model.ConstraintOperator;
import org.drools.model.Prototype;
import org.drools.model.PrototypeDSL;
Expand Down Expand Up @@ -63,8 +64,8 @@ private static ParsedCondition createSelectConditionWithLeftAndRightFields(RuleG
PrototypeDSL.PrototypePatternDef rightPattern = ruleContext.getBoundPattern(rightPatternBindName);

return rightPattern == null ?
new ParsedCondition(prototypeField(leftField), operator, prototypeField(rightField)) :
new BetaParsedCondition(prototypeField(leftField), operator, (PrototypeVariable) rightPattern.getFirstVariable(), prototypeField(rightField.substring(dotPos+1)));
new ParsedCondition(ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(leftField), operator, ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(rightField)) :
new BetaParsedCondition(ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(leftField), operator, (PrototypeVariable) rightPattern.getFirstVariable(), ExtractorPrototypeExpressionUtils.prototypeFieldExtractorSkippingFirst(rightField));
}

static ParsedCondition createSelectConditionWithFixedRight(ConditionExpression left, BiPredicate opPred, Object rhsValue, boolean positive) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
package org.drools.ansible.rulebook.integration.api.domain.temporal;

import org.drools.ansible.rulebook.integration.protoextractor.ExtractorParser;
import org.drools.ansible.rulebook.integration.protoextractor.ExtractorUtils;
import org.drools.ansible.rulebook.integration.protoextractor.ast.ExtractorNode;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpression;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpressionUtils;
import org.drools.base.facttemplates.Fact;
import org.drools.model.Index;
import org.drools.model.PrototypeDSL;
import org.drools.model.PrototypeExpression;
import org.drools.model.PrototypeFact;
import org.drools.model.PrototypeVariable;

import java.util.List;
Expand All @@ -18,13 +26,13 @@ public abstract class OnceAbstractTimeConstraint implements TimeConstraint {
protected String ruleName;

protected final TimeAmount timeAmount;
protected final List<String> groupByAttributes;
protected final List<GroupByAttribute> groupByAttributes;

protected PrototypeDSL.PrototypePatternDef guardedPattern;

private PrototypeVariable controlVariable;

public OnceAbstractTimeConstraint(TimeAmount timeAmount, List<String> groupByAttributes) {
public OnceAbstractTimeConstraint(TimeAmount timeAmount, List<GroupByAttribute> groupByAttributes) {
this.timeAmount = timeAmount;
this.groupByAttributes = groupByAttributes;
}
Expand All @@ -39,10 +47,13 @@ protected PrototypeVariable getControlVariable() {

protected PrototypeDSL.PrototypePatternDef createControlPattern() {
PrototypeDSL.PrototypePatternDef controlPattern = protoPattern(variable(getPrototype(SYNTHETIC_PROTOTYPE_NAME)));
for (String unique : groupByAttributes) {
controlPattern.expr( prototypeField(unique), Index.ConstraintType.EQUAL, getPatternVariable(), prototypeField(unique) );
for (GroupByAttribute unique : groupByAttributes) {
controlPattern.expr( prototypeField(unique.getKey()), // intentional, the control fact has the "group by" key string as-is (not structured), so we reference it for the left part
Index.ConstraintType.EQUAL,
getPatternVariable(),
unique.asPrototypeExpression() ); // on the right, we need extractor to check the real fact/event attribute value
}
controlPattern.expr( prototypeField("drools_rule_name"), Index.ConstraintType.EQUAL, fixedValue(ruleName) );
controlPattern.expr( ExtractorPrototypeExpressionUtils.prototypeFieldExtractor("drools_rule_name"), Index.ConstraintType.EQUAL, fixedValue(ruleName) );
this.controlVariable = (PrototypeVariable) controlPattern.getFirstVariable();
return controlPattern;
}
Expand All @@ -62,4 +73,39 @@ static String sanitizeAttributeName(String name) {
}
return name;
}

public static class GroupByAttribute {
private final String key;
private final ExtractorNode extractor;

private GroupByAttribute(String key, ExtractorNode extractor) {
this.key = key;
this.extractor = extractor;
}

public static GroupByAttribute from(String expr) {
return new GroupByAttribute(expr, ExtractorParser.parse(expr));
}

public String getKey() {
return key;
}

public PrototypeExpression asPrototypeExpression() {
return new ExtractorPrototypeExpression(extractor);
}

public Object evalExtractorOnFact(PrototypeFact fact) {
return evalExtractorOnFact((Fact) fact);
}

public Object evalExtractorOnFact(Fact fact) {
return ExtractorUtils.getValueFrom(extractor, fact.asMap());
}

@Override
public String toString() {
return "GroupByAttribute [key=" + key + ", extractor=" + extractor + "]";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private static Object controlFact2Event(Fact fact) {
return writeMetaDataOnEvent((Fact) fact.get("event"), ruleEngineMeta);
}

public OnceAfterDefinition(TimeAmount timeAmount, List<String> groupByAttributes) {
public OnceAfterDefinition(TimeAmount timeAmount, List<GroupByAttribute> groupByAttributes) {
super(timeAmount, groupByAttributes);
}

Expand Down Expand Up @@ -194,8 +194,8 @@ public List<Rule> getControlRules(RuleGenerationContext ruleContext) {
not( createControlPattern() ),
on(getPatternVariable()).execute((drools, event) -> {
Event controlEvent = createMapBasedEvent( controlPrototype );
for (String unique : groupByAttributes) {
controlEvent.set(unique, event.get(unique));
for (GroupByAttribute unique : groupByAttributes) {
controlEvent.set(unique.getKey(), unique.evalExtractorOnFact(event));
}
controlEvent.set("drools_rule_name", ruleName);
controlEvent.set( "event", event );
Expand Down Expand Up @@ -250,7 +250,10 @@ public String toString() {
}

public static OnceAfterDefinition parseOnceAfter(String onceWithin, List<String> groupByAttributes) {
List<String> sanitizedAttributes = groupByAttributes.stream().map(OnceAbstractTimeConstraint::sanitizeAttributeName).collect(toList());
List<GroupByAttribute> sanitizedAttributes = groupByAttributes.stream()
.map(OnceAbstractTimeConstraint::sanitizeAttributeName)
.map(GroupByAttribute::from)
.collect(toList());
return new OnceAfterDefinition(parseTimeAmount(onceWithin), sanitizedAttributes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public class OnceWithinDefinition extends OnceAbstractTimeConstraint {

public static final String KEYWORD = "once_within";

public OnceWithinDefinition(TimeAmount timeAmount, List<String> groupByAttributes) {
public OnceWithinDefinition(TimeAmount timeAmount, List<GroupByAttribute> groupByAttributes) {
super(timeAmount, groupByAttributes);
}

Expand All @@ -89,8 +89,8 @@ public void executeTimeConstraintConsequence(Drools drools, Object... facts) {
Event controlEvent = createMapBasedEvent( getPrototype(SYNTHETIC_PROTOTYPE_NAME) )
.withExpiration(timeAmount.getAmount(), timeAmount.getTimeUnit());
Fact fact = (Fact) facts[0];
for (String unique : groupByAttributes) {
controlEvent.set(unique, fact.get(unique));
for (GroupByAttribute unique : groupByAttributes) {
controlEvent.set(unique.getKey(), unique.evalExtractorOnFact(fact));
}
controlEvent.set("drools_rule_name", ruleName);
drools.insert(controlEvent);
Expand Down Expand Up @@ -127,7 +127,10 @@ public String toString() {
}

public static OnceWithinDefinition parseOnceWithin(String onceWithin, List<String> groupByAttributes) {
List<String> sanitizedAttributes = groupByAttributes.stream().map(OnceAbstractTimeConstraint::sanitizeAttributeName).collect(toList());
List<GroupByAttribute> sanitizedAttributes = groupByAttributes.stream()
.map(OnceAbstractTimeConstraint::sanitizeAttributeName)
.map(GroupByAttribute::from)
.collect(toList());
return new OnceWithinDefinition(parseTimeAmount(onceWithin), sanitizedAttributes);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import org.drools.ansible.rulebook.integration.api.io.RuleExecutorChannel;
import org.drools.base.facttemplates.Fact;
import org.drools.core.common.InternalFactHandle;
import org.kie.api.runtime.KieSession;
import org.kie.api.runtime.rule.Match;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -247,4 +248,9 @@ private List<Match> atomicRuleEvaluation(boolean processEventInsertion, Supplier

return matches;
}

@Override
public KieSession asKieSession() {
return rulesExecutorSession.asKieSession();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.drools.ansible.rulebook.integration.api.rulesengine;

import org.drools.ansible.rulebook.integration.api.RulesExecutorContainer;
import org.kie.api.runtime.KieSession;
import org.kie.api.runtime.rule.Match;

import java.util.Collection;
Expand Down Expand Up @@ -41,4 +42,6 @@ public interface RulesEvaluator {
static RulesEvaluator createRulesEvaluator( RulesExecutorSession rulesExecutorSession, boolean async ) {
return async ? new AsyncRulesEvaluator(rulesExecutorSession) : new SyncRulesEvaluator(rulesExecutorSession);
}

KieSession asKieSession();
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.util.function.BiPredicate;
import java.util.stream.Collectors;

import static org.drools.ansible.rulebook.integration.api.rulesmodel.RulesModelUtil.ORIGINAL_MAP_FIELD;
import static org.drools.ansible.rulebook.integration.api.rulesmodel.RulesModelUtil.mapToFact;


Expand Down Expand Up @@ -107,7 +106,7 @@ private static boolean isKeyToBeIgnored(String wmFactKey, String... keysToExclud
}
}
}
return wmFactKey.equals(ORIGINAL_MAP_FIELD);
return false;
}

int fireAllRules() {
Expand Down Expand Up @@ -165,4 +164,8 @@ void setExecuteActions(boolean executeActions) {
public boolean isMatchMultipleRules() {
return rulesSet.isMatchMultipleRules();
}

public KieSession asKieSession() {
return kieSession;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import org.drools.ansible.rulebook.integration.api.domain.RuleGenerationContext;
import org.drools.ansible.rulebook.integration.api.domain.constraints.NegationOperator;
import org.drools.ansible.rulebook.integration.protoextractor.prototype.ExtractorPrototypeExpressionUtils;
import org.drools.model.ConstraintOperator;
import org.drools.model.PrototypeDSL;
import org.drools.model.PrototypeExpression;
Expand All @@ -22,7 +23,7 @@ public class ParsedCondition {
private boolean negated = false;

public ParsedCondition(String left, ConstraintOperator operator, Object right) {
this(prototypeField(left), operator, fixedValue(right));
this(ExtractorPrototypeExpressionUtils.prototypeFieldExtractor(left), operator, fixedValue(right));
}

public ParsedCondition(PrototypeExpression left, ConstraintOperator operator, PrototypeExpression right) {
Expand Down
Loading

0 comments on commit 28b41e5

Please sign in to comment.