This is an automated email from the ASF dual-hosted git repository.
tzimanyi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie-drools.git
The following commit(s) were added to refs/heads/main by this push:
new bd28a79070 [incubator-kie-drools-6016] Emit a warning when an eval is
improperly and unnecessarily used (#6024)
bd28a79070 is described below
commit bd28a79070d61ca697bd54bd8fbd8f8157d60ab4
Author: Toshiya Kobayashi <[email protected]>
AuthorDate: Mon Jul 29 20:00:29 2024 +0900
[incubator-kie-drools-6016] Emit a warning when an eval is improperly and
unnecessarily used (#6024)
---
.../java/org/drools/base/rule/EvalCondition.java | 39 ++++++++++++++++++++++
.../drools/modelcompiler/KiePackagesBuilder.java | 5 ++-
.../drools/mvel/asm/AbstractASMEvalBuilder.java | 2 ++
.../org/drools/mvel/builder/MVELEvalBuilder.java | 2 ++
4 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drools-base/src/main/java/org/drools/base/rule/EvalCondition.java
b/drools-base/src/main/java/org/drools/base/rule/EvalCondition.java
index aca624bb86..3a979344b6 100644
--- a/drools-base/src/main/java/org/drools/base/rule/EvalCondition.java
+++ b/drools-base/src/main/java/org/drools/base/rule/EvalCondition.java
@@ -32,11 +32,18 @@ import org.drools.base.reteoo.BaseTuple;
import org.drools.base.rule.accessor.CompiledInvoker;
import org.drools.base.rule.accessor.EvalExpression;
import org.drools.base.rule.accessor.Wireable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
public class EvalCondition extends ConditionalElement
implements
Externalizable,
Wireable {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(EvalCondition.class);
+
+ private static long warnLogCounter = 0;
+
private static final long serialVersionUID = 510l;
protected EvalExpression expression;
@@ -223,4 +230,36 @@ public class EvalCondition extends ConditionalElement
public String toString() {
return this.expression.toString();
}
+
+ public static void logWarnIfImproperEval(EvalCondition evalCondition,
String evalExpression) {
+ if (warnLogCounter == 10) {
+ warnLogCounter++;
+ LOG.warn("More eval warnings will be suppressed...");
+ return;
+ } else if (warnLogCounter > 10) {
+ return; // avoid flooding the logs
+ }
+
+ if (evalExpression == null || evalExpression.isEmpty()) {
+ return; // cannot provide a meaningful warning
+ }
+
+ StringBuilder sb = new StringBuilder();
+ for (Declaration declaration :
evalCondition.getRequiredDeclarations()) {
+ if (declaration.getPattern() != null) {
+ sb.append("'");
+ sb.append(declaration.getIdentifier());
+ sb.append("' comes from previous pattern '");
+ String className =
declaration.getPattern().getObjectType().getClassName();
+ sb.append(className.substring(className.lastIndexOf('.') + 1));
+ sb.append("'. ");
+ }
+ }
+ if (!sb.isEmpty()) {
+ warnLogCounter++;
+ LOG.warn("In an eval expression [{}] : {}" +
+ "Consider placing the constraint in the pattern
and removing the eval if possible," +
+ " as eval is not performance-efficient.",
evalExpression, sb);
+ }
+ }
}
diff --git
a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/KiePackagesBuilder.java
b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/KiePackagesBuilder.java
index 3189e751f7..7c4a0af6e8 100644
---
a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/KiePackagesBuilder.java
+++
b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/KiePackagesBuilder.java
@@ -151,6 +151,7 @@ import
org.kie.internal.builder.KnowledgeBuilderConfiguration;
import org.kie.internal.builder.conf.PropertySpecificOption;
import static java.util.stream.Collectors.toList;
+import static org.drools.base.rule.EvalCondition.logWarnIfImproperEval;
import static org.drools.base.rule.GroupElement.AND;
import static org.drools.base.rule.GroupElement.OR;
import static org.drools.compiler.rule.builder.RuleBuilder.buildTimer;
@@ -669,7 +670,9 @@ public class KiePackagesBuilder {
private EvalCondition buildEval(RuleContext ctx, EvalImpl eval) {
Declaration[] declarations = Stream.of( eval.getExpr().getVariables()
).map( ctx::getDeclaration ).toArray( Declaration[]::new );
EvalExpression evalExpr = new LambdaEvalExpression(declarations,
eval.getExpr());
- return new EvalCondition(evalExpr, declarations);
+ EvalCondition evalCondition = new EvalCondition(evalExpr,
declarations);
+ logWarnIfImproperEval(evalCondition,
eval.getExpr().predicateInformation().getStringConstraint());
+ return evalCondition;
}
private ConditionalBranch buildConditionalConsequence(RuleContext ctx,
ConditionalNamedConsequenceImpl consequence) {
diff --git
a/drools-mvel/src/main/java/org/drools/mvel/asm/AbstractASMEvalBuilder.java
b/drools-mvel/src/main/java/org/drools/mvel/asm/AbstractASMEvalBuilder.java
index 8e251f3cdb..8ade06cbce 100644
--- a/drools-mvel/src/main/java/org/drools/mvel/asm/AbstractASMEvalBuilder.java
+++ b/drools-mvel/src/main/java/org/drools/mvel/asm/AbstractASMEvalBuilder.java
@@ -38,6 +38,7 @@ import org.drools.base.rule.Pattern;
import org.drools.base.rule.RuleConditionElement;
import org.drools.base.rule.accessor.DeclarationScopeResolver;
+import static org.drools.base.rule.EvalCondition.logWarnIfImproperEval;
import static org.drools.compiler.rule.builder.PatternBuilder.buildAnalysis;
import static
org.drools.compiler.rule.builder.PatternBuilder.createImplicitBindings;
import static org.drools.mvel.java.JavaRuleBuilderHelper.createVariableContext;
@@ -90,6 +91,7 @@ public abstract class AbstractASMEvalBuilder implements
RuleConditionBuilder {
Arrays.sort(declarations, SortDeclarations.instance);
EvalCondition eval =
EvalConditionFactory.Factory.get().createEvalCondition(declarations);
+ logWarnIfImproperEval(eval, (String) evalDescr.getContent());
Map<String, Object> vars = createVariableContext( className,
(String)evalDescr.getContent(),
diff --git
a/drools-mvel/src/main/java/org/drools/mvel/builder/MVELEvalBuilder.java
b/drools-mvel/src/main/java/org/drools/mvel/builder/MVELEvalBuilder.java
index db6ccbead3..0859f5a901 100644
--- a/drools-mvel/src/main/java/org/drools/mvel/builder/MVELEvalBuilder.java
+++ b/drools-mvel/src/main/java/org/drools/mvel/builder/MVELEvalBuilder.java
@@ -40,6 +40,7 @@ import org.drools.mvel.MVELDialectRuntimeData;
import org.drools.mvel.expr.MVELCompilationUnit;
import org.drools.mvel.expr.MVELEvalExpression;
+import static org.drools.base.rule.EvalCondition.logWarnIfImproperEval;
import static org.drools.mvel.asm.AsmUtil.copyErrorLocation;
public class MVELEvalBuilder
@@ -99,6 +100,7 @@ public class MVELEvalBuilder
false,
MVELCompilationUnit.Scope.EXPRESSION );
final EvalCondition eval =
EvalConditionFactory.Factory.get().createEvalCondition( previousDeclarations );
+ logWarnIfImproperEval(eval, (String) evalDescr.getContent());
MVELEvalExpression expr = new MVELEvalExpression( unit,
dialect.getId()
);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
