This is an automated email from the ASF dual-hosted git repository.
tzimanyi pushed a commit to branch 8.40.x
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/8.40.x by this push:
new bafc209d34 [kie-issues#969]: Negated value in accumulate function ends
with Invalid expression error (#5762) (#5772)
bafc209d34 is described below
commit bafc209d3437048ca6bb18f7497b18c485d6ca19
Author: Yeser Amer <[email protected]>
AuthorDate: Thu Mar 7 08:55:24 2024 +0100
[kie-issues#969]: Negated value in accumulate function ends with Invalid
expression error (#5762) (#5772)
---
.../visitor/accumulate/AccumulateVisitor.java | 21 ++++--
.../model/codegen/execmodel/AccumulateTest.java | 77 ++++++++++++++++++++++
.../drools/mvel/parser/DroolsMvelParserTest.java | 18 +++++
3 files changed, 109 insertions(+), 7 deletions(-)
diff --git
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/visitor/accumulate/AccumulateVisitor.java
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/visitor/accumulate/AccumulateVisitor.java
index 21bbb62d1c..f8ba516bae 100644
---
a/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/visitor/accumulate/AccumulateVisitor.java
+++
b/drools-model/drools-model-codegen/src/main/java/org/drools/model/codegen/execmodel/generator/visitor/accumulate/AccumulateVisitor.java
@@ -36,6 +36,7 @@ import com.github.javaparser.ast.expr.LiteralExpr;
import com.github.javaparser.ast.expr.MethodCallExpr;
import com.github.javaparser.ast.expr.MethodReferenceExpr;
import com.github.javaparser.ast.expr.NameExpr;
+import com.github.javaparser.ast.expr.UnaryExpr;
import com.github.javaparser.ast.stmt.BlockStmt;
import com.github.javaparser.ast.stmt.ExpressionStmt;
import org.drools.drl.ast.descr.AccumulateDescr;
@@ -179,8 +180,11 @@ public class AccumulateVisitor {
if ( function.getParams().length == 0 ) {
final AccumulateFunction optAccumulateFunction =
getAccumulateFunction( function, Object.class );
zeroParameterFunction( basePattern, functionDSL, bindingId,
optAccumulateFunction );
- } else {
+ } else if (function.getParams().length == 1) {
optNewBinding = parseFirstParameter( basePattern, input,
function, functionDSL, bindingId );
+ } else {
+ throw new AccumulateParsingFailedException(
+ "Function \"" + function.getFunction() + "\" cannot
have more than 1 parameter");
}
if ( bindingId != null ) {
@@ -199,8 +203,8 @@ public class AccumulateVisitor {
final String accumulateFunctionParameterStr = function.getParams()[0];
final Expression accumulateFunctionParameter =
DrlxParseUtil.parseExpression(accumulateFunctionParameterStr).getExpr();
- if (accumulateFunctionParameter instanceof BinaryExpr) {
- return binaryExprParameter(basePattern, function, functionDSL,
bindingId, accumulateFunctionParameterStr);
+ if (accumulateFunctionParameter instanceof BinaryExpr ||
accumulateFunctionParameter instanceof UnaryExpr) {
+ return bindingParameter(basePattern, function, functionDSL,
bindingId, accumulateFunctionParameterStr);
}
if
(parameterNeedsConvertionToMethodCallExpr(accumulateFunctionParameter)) {
@@ -212,7 +216,11 @@ public class AccumulateVisitor {
} else if (accumulateFunctionParameter instanceof LiteralExpr) {
literalExprParameter(basePattern, function, functionDSL,
bindingId, accumulateFunctionParameter);
} else {
- throw new AccumulateParsingFailedException("Invalid expression " +
accumulateFunctionParameterStr);
+ throw new AccumulateParsingFailedException(
+ "The expression \"" + accumulateFunctionParameterStr +
+ "\" in function \"" + function.getFunction() +
+ "\" of type \"" +
accumulateFunctionParameter.getClass().getSimpleName() +
+ "\" is not managed in " + this.getClass().getSimpleName());
}
return Optional.empty();
@@ -392,10 +400,10 @@ public class AccumulateVisitor {
return accumulateFunctionParameter.isMethodCallExpr() ||
accumulateFunctionParameter.isArrayAccessExpr() ||
accumulateFunctionParameter.isFieldAccessExpr();
}
- private Optional<NewBinding> binaryExprParameter(PatternDescr basePattern,
AccumulateDescr.AccumulateFunctionCallDescr function, MethodCallExpr
functionDSL, String bindingId, String accumulateFunctionParameterStr) {
+ private Optional<NewBinding> bindingParameter(PatternDescr basePattern,
AccumulateDescr.AccumulateFunctionCallDescr function, MethodCallExpr
functionDSL, String bindingId, String accumulateFunctionParameterStr) {
final DrlxParseResult parseResult =
ConstraintParser.defaultConstraintParser(context,
packageModel).drlxParse(Object.class, bindingId,
accumulateFunctionParameterStr);
- Optional<NewBinding> optNewBinding =
parseResult.acceptWithReturnValue(new
ParseResultVisitor<Optional<NewBinding>>() {
+ return parseResult.acceptWithReturnValue(new ParseResultVisitor<>() {
@Override
public Optional<NewBinding> onSuccess(DrlxParseSuccess
drlxParseResult) {
SingleDrlxParseSuccess singleResult = (SingleDrlxParseSuccess)
drlxParseResult;
@@ -422,7 +430,6 @@ public class AccumulateVisitor {
return Optional.empty();
}
});
- return optNewBinding;
}
private void zeroParameterFunction(PatternDescr basePattern,
MethodCallExpr functionDSL, String bindingId, AccumulateFunction
accumulateFunction) {
diff --git
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
index 098a8bac7a..fcc38a5330 100644
---
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
+++
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
@@ -58,6 +58,8 @@ import org.drools.model.codegen.execmodel.domain.StockTick;
import org.drools.model.codegen.execmodel.domain.TargetPolicy;
import org.drools.model.codegen.execmodel.oopathdtables.InternationalAddress;
import org.junit.Test;
+import org.kie.api.builder.Message;
+import org.kie.api.builder.Results;
import org.kie.api.definition.type.FactType;
import org.kie.api.runtime.KieSession;
import org.kie.api.runtime.rule.AccumulateFunction;
@@ -4202,4 +4204,79 @@ public class AccumulateTest extends BaseModelTest {
assertThat(results.size()).isEqualTo(2);
assertThat(results).containsExactlyInAnyOrder(new Result(0), new
Result(75));
}
+
+ @Test
+ public void testAccumulateSumMultipleParametersExpression() {
+ String str =
+ "import " + Person.class.getCanonicalName() + ";" +
+ "import " + Result.class.getCanonicalName() + ";" +
+ "rule X when\n" +
+ " accumulate ( $i : Integer() and $p: Person ( name.length >=
$i ); \n" +
+ " $sum : sum($p.getAge(), $p.getName()) \n" +
+ " ) \n" +
+ "then\n" +
+ " insert(new Result($sum));\n" +
+ "end";
+
+ Results results = createKieBuilder(str).getResults();
+ if (testRunType.isExecutableModel()) {
+
assertThat(results.getMessages(Message.Level.ERROR).get(0).getText().contains(
+ "Function \"sum\" cannot have more than 1
parameter")).isTrue();
+ } else {
+ // At this time, this error is thrown with executable model only.
+
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isTrue();
+ }
+ }
+
+ @Test
+ public void testAccumulateSumUnaryExpression() {
+ String str =
+ "import " + Person.class.getCanonicalName() + ";" +
+ "import " + Result.class.getCanonicalName() + ";" +
+ "rule X when\n" +
+ " accumulate ( $p: Person ( getName().startsWith(\"M\") );
\n" +
+ " $sum : sum(-$p.getAge()) \n" +
+ " ) \n" +
+ "then\n" +
+ " insert(new Result($sum));\n" +
+ "end";
+
+ KieSession kieSession = getKieSession( str );
+
+ kieSession.insert(new Person("Mark", 37));
+ kieSession.insert(new Person("Edson", 35));
+ kieSession.insert(new Person("Mario", 40));
+
+ kieSession.fireAllRules();
+
+ Collection<Result> results = getObjectsIntoList(kieSession,
Result.class);
+ assertThat(results.size()).isEqualTo(1);
+ assertThat(results.iterator().next().getValue()).isEqualTo(-77);
+ }
+
+ @Test
+ public void testAccumulateSumBinaryExpWithNestedUnaryExpression() {
+ String str =
+ "import " + Person.class.getCanonicalName() + ";" +
+ "import " + Result.class.getCanonicalName() + ";" +
+ "rule X when\n" +
+ " accumulate ( $p: Person ( getName().startsWith(\"M\") );
\n" +
+ " $sum : sum(-$p.getAge() + $p.getAge()) \n" +
+ " ) \n" +
+ "then\n" +
+ " insert(new Result($sum));\n" +
+ "end";
+
+ KieSession kieSession = getKieSession( str );
+
+ kieSession.insert(new Person("Mark", 37));
+ kieSession.insert(new Person("Edson", 35));
+ kieSession.insert(new Person("Mario", 40));
+
+ kieSession.fireAllRules();
+
+ Collection<Result> results = getObjectsIntoList(kieSession,
Result.class);
+ assertThat(results.size()).isEqualTo(1);
+ assertThat(results.iterator().next().getValue()).isEqualTo(0);
+ }
}
diff --git
a/drools-model/drools-mvel-parser/src/test/java/org/drools/mvel/parser/DroolsMvelParserTest.java
b/drools-model/drools-mvel-parser/src/test/java/org/drools/mvel/parser/DroolsMvelParserTest.java
index c86761f0e0..cfe06bf87b 100644
---
a/drools-model/drools-mvel-parser/src/test/java/org/drools/mvel/parser/DroolsMvelParserTest.java
+++
b/drools-model/drools-mvel-parser/src/test/java/org/drools/mvel/parser/DroolsMvelParserTest.java
@@ -188,6 +188,22 @@ public class DroolsMvelParserTest {
assertThat(printNode(expression)).isEqualTo(expr);
}
+ @Test
+ public void testConstantUnaryExpression() {
+ String expr = "-49";
+ Expression expression = parseExpression( parser, expr ).getExpr();
+ assertThat(printNode(expression)).isEqualTo(expr);
+ assertThat(expression.isUnaryExpr()).isTrue();
+ }
+
+ @Test
+ public void testVariableUnaryExpression() {
+ String expr = "-$a";
+ Expression expression = parseExpression( parser, expr ).getExpr();
+ assertThat(printNode(expression)).isEqualTo(expr);
+ assertThat(expression.isUnaryExpr()).isTrue();
+ }
+
@Test
public void testDotFreeEnclosedWithNameExpr() {
String expr = "(something after $a)";
@@ -408,10 +424,12 @@ public class DroolsMvelParserTest {
BinaryExpr first = (BinaryExpr) comboExpr.getLeft();
assertThat(toString(first.getLeft())).isEqualTo("value");
assertThat(toString(first.getRight())).isEqualTo("-2");
+ assertThat(first.getRight().isUnaryExpr()).isTrue();
assertThat(first.getOperator()).isEqualTo(Operator.GREATER);
HalfBinaryExpr second = (HalfBinaryExpr) comboExpr.getRight();
assertThat(toString(second.getRight())).isEqualTo("-1");
+ assertThat(second.getRight().isUnaryExpr()).isTrue();
assertThat(second.getOperator()).isEqualTo(HalfBinaryExpr.Operator.LESS);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
