Skip to content

Commit

Permalink
[DROOLS-2545] Fix testUnchangedAccumulate (apache#1897)
Browse files Browse the repository at this point in the history 
* New reproducer from drools-test-coverage

* Better test with assertion

* Removed duplication and take the expression from left and result

* [DROOLS-2545] Fix testUnchangedAccumulate

* Better handling of target
  • Loading branch information
@lucamolteni @mariofusco
lucamolteni authored and mariofusco committed May 14, 2018
1 parent 0f2d292 commit f1939d0
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 23 deletions.
4 changes: 2 additions & 2 deletions .gitignore
View file
@@ -1,4 +1,4 @@
/target
target/
/local

# Eclipse, Netbeans and IntelliJ files
Expand All @@ -7,7 +7,7 @@
/nbproject
/*.ipr
/*.iws
/*.iml
*.iml

# Repository wide ignore mac DS_Store files
.DS_Store
Expand Down
28 changes: 28 additions & 0 deletions ...java/org/drools/modelcompiler/builder/generator/expression/AbstractExpressionBuilder.java
View file
Expand Up @@ -18,16 +18,20 @@

import java.util.Collection;

import org.drools.javaparser.ast.expr.EnclosedExpr;
import org.drools.javaparser.ast.expr.Expression;
import org.drools.javaparser.ast.expr.FieldAccessExpr;
import org.drools.javaparser.ast.expr.MethodCallExpr;
import org.drools.javaparser.ast.expr.NameExpr;
import org.drools.modelcompiler.builder.generator.DeclarationSpec;
import org.drools.modelcompiler.builder.generator.DrlxParseUtil;
import org.drools.modelcompiler.builder.generator.IndexIdGenerator;
import org.drools.modelcompiler.builder.generator.RuleContext;
import org.drools.modelcompiler.builder.generator.TypedExpression;
import org.drools.modelcompiler.builder.generator.drlxparse.DrlxParseSuccess;

import static org.drools.modelcompiler.builder.generator.DrlxParseUtil.generateLambdaWithoutParameters;

public abstract class AbstractExpressionBuilder {
protected static final IndexIdGenerator indexIdGenerator = new IndexIdGenerator();

Expand Down Expand Up @@ -64,6 +68,30 @@ private Expression buildUnificationExpression(DrlxParseSuccess drlxParseResult)

public abstract MethodCallExpr buildBinding(DrlxParseSuccess drlxParseResult );

protected Expression getConstraintExpression(DrlxParseSuccess drlxParseResult) {
if (drlxParseResult.getExpr() instanceof EnclosedExpr) {
return buildConstraintExpression(drlxParseResult, ((EnclosedExpr) drlxParseResult.getExpr()).getInner());
} else {
final TypedExpression left = drlxParseResult.getLeft();
// Can we unify it? Sometimes expression is in the left sometimes in expression
final Expression e;
if(left != null) {
e = DrlxParseUtil.findLeftLeafOfMethodCall(left.getExpression());
} else {
e = drlxParseResult.getExpr();
}
return buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarationsOnLeft(), e);
}
}

protected Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Expression expr ) {
return buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarations(), expr );
}

protected Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Collection<String> usedDeclarations, Expression expr ) {
return drlxParseResult.isStatic() ? expr : generateLambdaWithoutParameters(usedDeclarations, expr, drlxParseResult.isSkipThisAsParam());
}

boolean hasIndex( DrlxParseSuccess drlxParseResult ) {
TypedExpression left = drlxParseResult.getLeft();
Collection<String> usedDeclarations = drlxParseResult.getUsedDeclarations();
Expand Down
11 changes: 1 addition & 10 deletions ...ain/java/org/drools/modelcompiler/builder/generator/expression/FlowExpressionBuilder.java
View file
Expand Up @@ -81,11 +81,9 @@ public MethodCallExpr buildBinding(DrlxParseSuccess drlxParseResult ) {
} else {
bindDSL.addArgument( new NameExpr(toVar(drlxParseResult.getExprBinding())) );
}
final Expression constraintExpression = getConstraintExpression(drlxParseResult);
MethodCallExpr bindAsDSL = new MethodCallExpr(bindDSL, BIND_AS_CALL);
bindAsDSL.addArgument( new NameExpr(toVar(drlxParseResult.getPatternBinding())) );
final Expression constraintExpression = drlxParseResult.getExpr() instanceof EnclosedExpr ?
buildConstraintExpression(drlxParseResult, (( EnclosedExpr ) drlxParseResult.getExpr()).getInner()) :
buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarationsOnLeft(), DrlxParseUtil.findLeftLeafOfMethodCall(drlxParseResult.getLeft().getExpression()));
bindAsDSL.addArgument(constraintExpression);
return buildReactOn( drlxParseResult, bindAsDSL );
}
Expand All @@ -109,13 +107,6 @@ private MethodCallExpr buildReactOn(DrlxParseSuccess drlxParseResult, MethodCall
return exprDSL;
}

private Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Expression expr ) {
return buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarations(), expr );
}

private Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Collection<String> usedDeclarations, Expression expr ) {
return drlxParseResult.isStatic() ? expr : generateLambdaWithoutParameters(usedDeclarations, expr, drlxParseResult.isSkipThisAsParam());
}

private MethodCallExpr buildIndexedBy(DrlxParseSuccess drlxParseResult, MethodCallExpr exprDSL) {
if ( !hasIndex( drlxParseResult ) ) {
Expand Down
12 changes: 1 addition & 11 deletions .../java/org/drools/modelcompiler/builder/generator/expression/PatternExpressionBuilder.java
View file
Expand Up @@ -88,23 +88,13 @@ public MethodCallExpr buildBinding(DrlxParseSuccess drlxParseResult) {
} else {
bindDSL.addArgument(new NameExpr(toVar(drlxParseResult.getExprBinding())));
}
final Expression constraintExpression = drlxParseResult.getExpr() instanceof EnclosedExpr ?
buildConstraintExpression(drlxParseResult, (( EnclosedExpr ) drlxParseResult.getExpr()).getInner()) :
buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarationsOnLeft(), DrlxParseUtil.findLeftLeafOfMethodCall(drlxParseResult.getLeft().getExpression()));
final Expression constraintExpression = getConstraintExpression(drlxParseResult);
bindDSL.addArgument(constraintExpression);
final Optional<MethodCallExpr> methodCallExpr = buildReactOn(drlxParseResult);
methodCallExpr.ifPresent(bindDSL::addArgument);
return bindDSL;
}

private Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Expression expr) {
return buildConstraintExpression(drlxParseResult, drlxParseResult.getUsedDeclarations(), expr);
}

private Expression buildConstraintExpression(DrlxParseSuccess drlxParseResult, Collection<String> usedDeclarations, Expression expr) {
return drlxParseResult.isStatic() ? expr : generateLambdaWithoutParameters(usedDeclarations, expr, drlxParseResult.isSkipThisAsParam());
}

private Optional<MethodCallExpr> buildIndexedBy(DrlxParseSuccess drlxParseResult) {
if ( !hasIndex( drlxParseResult ) ) {
return Optional.empty();
Expand Down
29 changes: 29 additions & 0 deletions ...ls-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/AccumulateTest.java
View file
Expand Up @@ -677,4 +677,33 @@ public void testAccumulateFromWithConstraint() {
assertEquals(1, results.size());
assertThat(results, hasItem("Milan"));
}

@Test
public void testAccumulateWithThis() {
final String drl1 =
"import java.util.*;\n" +
"rule B\n" +
"when\n" +
" $eventCodeDistinctMois : Integer( intValue>0 ) from accumulate ( String( $id : this ),\n" +
" init( Set set = new HashSet(); ),\n" +
" action( set.add($id); ),\n" +
" reverse( set.remove($id); ),\n" +
" result( set.size()) )\n" +
"then\n" +
" insert($eventCodeDistinctMois);\n" +
"end";
KieSession ksession = getKieSession( drl1 );

ksession.insert("1");
ksession.insert("3");
ksession.insert("3");
ksession.insert("5");
ksession.insert("7");
ksession.fireAllRules();

List<Integer> results = getObjectsIntoList(ksession, Integer.class);

assertThat(results, hasItem(4));

}
}

0 comments on commit f1939d0

Please sign in to comment.