[DROOLS-2545] Fix testUnchangedAccumulate (#1897)

* 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
apache · May 14, 2018 · f1939d0 · f1939d0
1 parent 0f2d292
commit f1939d0
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 23 deletions.
diff --git a/.gitignore b/.gitignore
@@ -1,4 +1,4 @@
-/target
+target/
 /local
 
 # Eclipse, Netbeans and IntelliJ files
@@ -7,7 +7,7 @@
 /nbproject
 /*.ipr
 /*.iws
-/*.iml
+*.iml
 
 # Repository wide ignore mac DS_Store files
 .DS_Store

diff --git a/...java/org/drools/modelcompiler/builder/generator/expression/AbstractExpressionBuilder.java b/...java/org/drools/modelcompiler/builder/generator/expression/AbstractExpressionBuilder.java
@@ -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();
 
@@ -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();

diff --git a/...ain/java/org/drools/modelcompiler/builder/generator/expression/FlowExpressionBuilder.java b/...ain/java/org/drools/modelcompiler/builder/generator/expression/FlowExpressionBuilder.java
@@ -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 );
     }
@@ -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 ) ) {

diff --git a/.../java/org/drools/modelcompiler/builder/generator/expression/PatternExpressionBuilder.java b/.../java/org/drools/modelcompiler/builder/generator/expression/PatternExpressionBuilder.java
@@ -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();

diff --git a/...ls-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/AccumulateTest.java b/...ls-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/AccumulateTest.java
@@ -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));
+
+    }
 }