Skip to content

Commit

Permalink
[KIE-748] drools executable-model fails with BigDecimal coercion for … (
Browse files Browse the repository at this point in the history
apache#5618)

* [KIE-748] drools executable-model fails with BigDecimal coercion for method arguments

* refactor
  • Loading branch information
tkobayas authored and rgdoliveira committed Dec 13, 2023
1 parent 09dbd7f commit f12f3f6
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,7 @@ private DrlxParseResult parseBinaryExpr(BinaryExpr binaryExpr, Class<?> patternT

Expression combo;

boolean arithmeticExpr = ARITHMETIC_OPERATORS.contains(operator);
boolean arithmeticExpr = isArithmeticOperator(operator);
boolean isBetaConstraint = right.getExpression() != null && hasDeclarationFromOtherPattern( expressionTyperContext );
boolean requiresSplit = operator == BinaryExpr.Operator.AND && binaryExpr.getRight() instanceof HalfBinaryExpr && !isBetaConstraint;

Expand Down Expand Up @@ -981,7 +981,7 @@ private Optional<DrlxParseFail> convertBigDecimalArithmetic(MethodCallExpr metho
List<BinaryExpr> binaryExprList = methodCallExpr.findAll(BinaryExpr.class);
for (BinaryExpr binaryExpr : binaryExprList) {
Operator operator = binaryExpr.getOperator();
boolean arithmeticExpr = ARITHMETIC_OPERATORS.contains(operator);
boolean arithmeticExpr = isArithmeticOperator(operator);
if (arithmeticExpr) {
final ExpressionTyperContext expressionTyperContext = new ExpressionTyperContext();
final ExpressionTyper expressionTyper = new ExpressionTyper(context, patternType, bindingId, isPositional, expressionTyperContext);
Expand All @@ -1008,4 +1008,8 @@ private Optional<DrlxParseFail> convertBigDecimalArithmetic(MethodCallExpr metho
}
return Optional.empty();
}

public static boolean isArithmeticOperator(BinaryExpr.Operator operator) {
return ARITHMETIC_OPERATORS.contains(operator);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.lang.reflect.Modifier;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.TypeVariable;
import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -90,6 +91,8 @@
import org.drools.mvel.parser.ast.expr.OOPathExpr;
import org.drools.mvel.parser.ast.expr.PointFreeExpr;
import org.drools.mvel.parser.printer.PrintUtil;
import org.drools.mvelcompiler.CompiledExpressionResult;
import org.drools.mvelcompiler.ConstraintCompiler;
import org.drools.mvelcompiler.util.BigDecimalArgumentCoercion;
import org.drools.util.MethodUtils;
import org.drools.util.TypeResolver;
Expand All @@ -100,6 +103,7 @@
import static java.util.Optional.empty;
import static java.util.Optional.of;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.THIS_PLACEHOLDER;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.createConstraintCompiler;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.findRootNodeViaParent;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.getClassFromContext;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.getClassFromType;
Expand All @@ -114,6 +118,7 @@
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.toStringLiteral;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.transformDrlNameExprToNameExpr;
import static org.drools.model.codegen.execmodel.generator.DrlxParseUtil.trasformHalfBinaryToBinary;
import static org.drools.model.codegen.execmodel.generator.drlxparse.ConstraintParser.isArithmeticOperator;
import static org.drools.model.codegen.execmodel.generator.expressiontyper.FlattenScope.flattenScope;
import static org.drools.model.codegen.execmodel.generator.expressiontyper.FlattenScope.transformFullyQualifiedInlineCastExpr;
import static org.drools.mvel.parser.MvelParser.parseType;
Expand Down Expand Up @@ -801,7 +806,30 @@ private TypedExpressionCursor binaryExpr(BinaryExpr binaryExpr) {
TypedExpression rightTypedExpression = right.getTypedExpression()
.orElseThrow(() -> new NoSuchElementException("TypedExpressionResult doesn't contain TypedExpression!"));
binaryExpr.setRight(rightTypedExpression.getExpression());
return new TypedExpressionCursor(binaryExpr, getBinaryType(leftTypedExpression, rightTypedExpression, binaryExpr.getOperator()));
java.lang.reflect.Type binaryType = getBinaryType(leftTypedExpression, rightTypedExpression, binaryExpr.getOperator());
if (shouldConvertArithmeticBinaryToMethodCall(binaryExpr.getOperator(), binaryType)) {
Expression compiledExpression = convertArithmeticBinaryToMethodCall(binaryExpr, leftTypedExpression.getOriginalPatternType(), ruleContext);
return new TypedExpressionCursor(compiledExpression, binaryType);
} else {
return new TypedExpressionCursor(binaryExpr, binaryType);
}
}

/*
* Converts arithmetic binary expression (including coercion) to method call using ConstraintCompiler.
* This method can be generic, so we may centralize the calls in drools-model
*/
private static Expression convertArithmeticBinaryToMethodCall(BinaryExpr binaryExpr, Optional<Class<?>> originalPatternType, RuleContext ruleContext) {
ConstraintCompiler constraintCompiler = createConstraintCompiler(ruleContext, originalPatternType);
CompiledExpressionResult compiledExpressionResult = constraintCompiler.compileExpression(printNode(binaryExpr));
return compiledExpressionResult.getExpression();
}

/*
* BigDecimal arithmetic operations should be converted to method calls. We may also apply this to BigInteger.
*/
private static boolean shouldConvertArithmeticBinaryToMethodCall(BinaryExpr.Operator operator, java.lang.reflect.Type type) {
return isArithmeticOperator(operator) && type.equals(BigDecimal.class);
}

private java.lang.reflect.Type getBinaryType(TypedExpression leftTypedExpression, TypedExpression rightTypedExpression, Operator operator) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
* <p>
* http://www.apache.org/licenses/LICENSE-2.0
* <p>
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.drools.model.codegen.execmodel.bigdecimaltest;

import java.math.BigDecimal;

public class BDFact {

private BigDecimal value1;
private BigDecimal value2;

public BigDecimal getValue1() {
return value1;
}

public void setValue1(BigDecimal value1) {
this.value1 = value1;
}

public BigDecimal getValue2() {
return value2;
}

public void setValue2(BigDecimal value2) {
this.value2 = value2;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -721,4 +721,55 @@ public void bigDecimalEqualityWithDifferentScale_shouldBeEqual() {
// BigDecimal("1.0") and BigDecimal("1.00") are considered as equal because exec-model uses EvaluationUtil.equals() which is based on compareTo()
assertThat(result).contains(new BigDecimal("1.00"));
}

@Test
public void bigDecimalCoercionInMethodArgument_shouldNotFailToBuild() {
// KIE-748
String str =
"package org.drools.modelcompiler.bigdecimals\n" +
"import " + BDFact.class.getCanonicalName() + ";\n" +
"import static " + BigDecimalTest.class.getCanonicalName() + ".intToString;\n" +
"rule \"Rule 1a\"\n" +
" when\n" +
" BDFact( intToString(value2 - 1) == \"2\" )\n" +
" then\n" +
"end";

KieSession ksession = getKieSession(str);

BDFact bdFact = new BDFact();
bdFact.setValue2(new BigDecimal("3"));

ksession.insert(bdFact);

assertThat(ksession.fireAllRules()).isEqualTo(1);
}

@Test
public void bigDecimalCoercionInNestedMethodArgument_shouldNotFailToBuild() {
// KIE-748
String str =
"package org.drools.modelcompiler.bigdecimals\n" +
"import " + BDFact.class.getCanonicalName() + ";\n" +
"import static " + BigDecimalTest.class.getCanonicalName() + ".intToString;\n" +
"rule \"Rule 1a\"\n" +
" when\n" +
" BDFact( intToString(value1 * (value2 - 1)) == \"20\" )\n" +
" then\n" +
"end";

KieSession ksession = getKieSession(str);

BDFact bdFact = new BDFact();
bdFact.setValue1(new BigDecimal("10"));
bdFact.setValue2(new BigDecimal("3"));

ksession.insert(bdFact);

assertThat(ksession.fireAllRules()).isEqualTo(1);
}

public static String intToString(int value) {
return Integer.toString(value);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ public void parseIntStringConcatenation() {
assertThat(typedExpression.getExpression().toString()).isEqualTo(expected);
}

@Test
public void coercionInMethodArgument() {
TypedExpression typedExpression = toTypedExpression("identityBigDecimal(money - 1)", Person.class);
assertThat(ruleContext.hasCompilationError()).isFalse();
String expected = "_this.identityBigDecimal(_this.getMoney().subtract(new java.math.BigDecimal(1), java.math.MathContext.DECIMAL128))";
assertThat(typedExpression.getExpression().toString()).isEqualTo(expected);
}

private TypedExpression toTypedExpression(String inputExpression, Class<?> patternType, DeclarationSpec... declarations) {

for(DeclarationSpec d : declarations) {
Expand Down

0 comments on commit f12f3f6

Please sign in to comment.