Skip to content

Commit

Permalink
[7.67.x] [DROOLS-7493] [DROOLS-7497] executable model wrongly rewrite…
Browse files Browse the repository at this point in the history 
…s modify in if-block (#8) (#9)

* [DROOLS-7195] Modify syntax fails when using executable model, works … (apache#4846) (apache#4868)

* [DROOLS-7195] Modify syntax fails when using executable model, works with mvel runtime (nested properties)

* - Dropping 'with' statement support

* [DROOLS-7493] [DROOLS-7497] executable model wrongly rewrites modify in if-block (apache#5380)

* [DROOLS-7493] executable model wrongly rewrites modify in if-block
- Additional tests to cover setter order for properperty reactivity [DROOLS-7497]

* - analyze whole RHS and make all modification as property reactive

* - Add docs about fact modification after modify or update in RHS

* - fixing code smells
  • Loading branch information
@tkobayas
tkobayas authored Oct 16, 2023
1 parent 2b151b7 commit 107e91d
Show file tree
Hide file tree
Showing 13 changed files with 580 additions and 223 deletions.
29 changes: 9 additions & 20 deletions ...-model-compiler/src/main/java/org/drools/modelcompiler/builder/generator/Consequence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -65,7 +64,6 @@
import org.drools.mvelcompiler.MvelCompilerException;

import static com.github.javaparser.StaticJavaParser.parseExpression;
import static com.github.javaparser.ast.NodeList.nodeList;
import static java.util.stream.Collectors.toSet;
import static org.drools.core.util.ClassUtils.getter2property;
import static org.drools.core.util.ClassUtils.isGetter;
Expand Down Expand Up @@ -152,7 +150,7 @@ public MethodCallExpr createCall(RuleDescr ruleDescr, String consequenceString,
switch (context.getRuleDialect()) {
case JAVA:
rewriteReassignedDeclarations(ruleConsequence, usedDeclarationInRHS);
executeCall = executeCall(ruleVariablesBlock, ruleConsequence, usedDeclarationInRHS, onCall, Collections.emptySet());
executeCall = executeCall(ruleVariablesBlock, ruleConsequence, usedDeclarationInRHS, onCall);
break;
case MVEL:
executeCall = createExecuteCallMvel(consequenceString, ruleVariablesBlock, usedDeclarationInRHS, onCall);
Expand Down Expand Up @@ -208,9 +206,9 @@ private MethodCallExpr createExecuteCallMvel(String consequenceString, BlockStmt
return executeCall(ruleVariablesBlock,
compile.statementResults(),
usedDeclarationInRHS,
onCall,
compile.getUsedBindings());
onCall);
}

private BlockStmt rewriteConsequence(String consequence) {
try {
String ruleConsequenceAsBlock = rewriteModifyBlock(consequence.trim());
Expand Down Expand Up @@ -267,16 +265,7 @@ public static boolean containsWord(String word, String body) {
return m.find();
}

private MethodCallExpr executeCall(BlockStmt ruleVariablesBlock, BlockStmt ruleConsequence, Collection<String> verifiedDeclUsedInRHS, MethodCallExpr onCall, Set<String> modifyProperties) {

for (String modifiedProperty : modifyProperties) {
NodeList<Expression> arguments = nodeList(new NameExpr(modifiedProperty));
MethodCallExpr update = new MethodCallExpr(new NameExpr("drools"), "update",
arguments);
ruleConsequence.getStatements().add(new ExpressionStmt(update));
}


private MethodCallExpr executeCall(BlockStmt ruleVariablesBlock, BlockStmt ruleConsequence, Collection<String> verifiedDeclUsedInRHS, MethodCallExpr onCall) {
boolean requireDrools = rewriteRHS(ruleVariablesBlock, ruleConsequence);
MethodCallExpr executeCall = new MethodCallExpr(onCall != null ? onCall : new NameExpr("D"), EXECUTE_CALL);
LambdaExpr executeLambda = new LambdaExpr();
Expand Down Expand Up @@ -360,8 +349,8 @@ private boolean rewriteRHS(BlockStmt ruleBlock, BlockStmt rhs) {
if (context.isPropertyReactive(updatedClass)) {

if ( !initializedBitmaskFields.contains( updatedVar ) ) {
Set<String> modifiedProps = findModifiedProperties( methodCallExprs, updateExpr, updatedVar, updatedClass );
modifiedProps.addAll(findModifiedPropertiesFromAssignment( assignExprs, updateExpr, updatedVar, updatedClass ));
Set<String> modifiedProps = findModifiedProperties(methodCallExprs, updatedVar, updatedClass );
modifiedProps.addAll(findModifiedPropertiesFromAssignment(assignExprs, updatedVar));
MethodCallExpr bitMaskCreation = createBitMaskInitialization( updatedClass, modifiedProps );
AssignExpr bitMaskAssign = createBitMaskField(updatedVar, bitMaskCreation);
if (!DrlxParseUtil.hasDuplicateExpr(ruleBlock, bitMaskAssign)) {
Expand Down Expand Up @@ -411,9 +400,9 @@ private AssignExpr createBitMaskField(String updatedVar, MethodCallExpr bitMaskC
return new AssignExpr(bitMaskVar, bitMaskCreation, AssignExpr.Operator.ASSIGN);
}

private Set<String> findModifiedProperties( List<MethodCallExpr> methodCallExprs, MethodCallExpr updateExpr, String updatedVar, Class<?> updatedClass ) {
private Set<String> findModifiedProperties( List<MethodCallExpr> methodCallExprs, String updatedVar, Class<?> updatedClass ) {
Set<String> modifiedProps = new HashSet<>();
for (MethodCallExpr methodCall : methodCallExprs.subList(0, methodCallExprs.indexOf(updateExpr))) {
for (MethodCallExpr methodCall : methodCallExprs) {
if (!isDirectExpression(methodCall)) {
continue; // don't evaluate a method which is a part of other expression
}
Expand Down Expand Up @@ -463,7 +452,7 @@ private Set<String> findModifiedProperties( List<MethodCallExpr> methodCallExprs
return modifiedProps;
}

private Set<String> findModifiedPropertiesFromAssignment(List<AssignExpr> assignExprs, MethodCallExpr updateExpr, String updatedVar, Class<?> updatedClass) {
private Set<String> findModifiedPropertiesFromAssignment(List<AssignExpr> assignExprs, String updatedVar) {
Set<String> modifiedProps = new HashSet<>();
for (AssignExpr assignExpr : assignExprs) {
Expression target = assignExpr.getTarget();
Expand Down
91 changes: 86 additions & 5 deletions ...drools-model-compiler/src/test/java/org/drools/modelcompiler/CompilationFailuresTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,21 +135,102 @@ private void checkCompilationFailureOnMismatchingAccumulate(String type, String


@Test
public void testModifyOnFactInScope() {
// DROOLS-5242
public void modify_factInScope_java() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1 when\n" +
"rule R1\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.setName(\"Mark\") }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

// RHS error : line = 1 with STANDARD_FROM_DRL (RuleDescr)
assertThat(results.getMessages().get(0).getLine()).isEqualTo(1);
@Test
public void modify_factInScope_mvel() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1\n" +
"dialect 'mvel'\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.setName(\"Mark\") }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

@Test
public void modify_factInScope_nestedPropertySetter_java() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.address.setCity(\"London\") }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

@Test
public void modify_factInScope_nestedPropertySetter_mvel() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1\n" +
"dialect 'mvel'\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.address.setCity(\"London\") }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

@Test
public void modify_factInScope_nestedPropertyAssign_java() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.address.city = \"London\" }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

@Test
public void modify_factInScope_nestedPropertyAssign_mvel() {
// DROOLS-5242, DROOLS-7195
String drl =
"import " + Person.class.getCanonicalName() + ";" +
"rule R1\n" +
"dialect 'mvel'\n" +
"when\n" +
" $p : Person(name == \"Mario\")\n" +
"then\n" +
" modify($p) { $p.address.city = \"London\" }\n" +
"end";

Results results = getCompilationResults(drl);
assertThat(results.getMessages(Message.Level.ERROR).isEmpty()).isFalse();
}

@Test
Expand Down
131 changes: 131 additions & 0 deletions ...s-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/MvelDialectTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.drools.core.WorkingMemory;
import org.drools.core.spi.Tuple;
import org.drools.modelcompiler.domain.Address;
import org.drools.modelcompiler.domain.Counter;
import org.drools.modelcompiler.domain.InternationalAddress;
import org.drools.modelcompiler.domain.Person;

Expand Down Expand Up @@ -1599,4 +1600,134 @@ public void drools_fieldAccess() {
assertThat(results.get("knowledgeRuntime")).isInstanceOf(KieRuntime.class);
assertThat(results.get("kieRuntime")).isInstanceOf(KieRuntime.class);
}

public void assign_nestedProperty() {
// DROOLS-7195
String str = "package com.example.reproducer\n" +
"import " + Person.class.getCanonicalName() + ";\n" +
"rule R\n" +
"dialect \"mvel\"\n" +
"when\n" +
" $p : Person()\n" +
"then\n" +
" $p.address.city = \"Tokyo\";\n" +
"end";

KieSession ksession = getKieSession(str);

Person p = new Person("John");
p.setAddress(new Address("London"));
ksession.insert(p);
ksession.fireAllRules();

assertThat(p.getAddress().getCity()).isEqualTo("Tokyo");
}

@Test
public void assign_nestedPropertyInModify() {
// DROOLS-7195
String str = "package com.example.reproducer\n" +
"import " + Person.class.getCanonicalName() + ";\n" +
"rule R\n" +
"dialect \"mvel\"\n" +
"when\n" +
" $p : Person(name == \"John\")\n" +
"then\n" +
" modify($p) {" +
" address.city = \"Tokyo\";\n" +
" }\n" +
"end";

KieSession ksession = getKieSession(str);

Person p = new Person("John");
p.setAddress(new Address("London"));
ksession.insert(p);
ksession.fireAllRules();

assertThat(p.getAddress().getCity()).isEqualTo("Tokyo");
}

@Test
public void setter_nestedPropertyInModify() {
// DROOLS-7195
String str = "package com.example.reproducer\n" +
"import " + Person.class.getCanonicalName() + ";\n" +
"rule R\n" +
"dialect \"mvel\"\n" +
"when\n" +
" $p : Person(name == \"John\")\n" +
"then\n" +
" modify($p) {" +
" address.setCity(\"Tokyo\");\n" +
" }\n" +
"end";

KieSession ksession = getKieSession(str);

Person p = new Person("John");
p.setAddress(new Address("London"));
ksession.insert(p);
ksession.fireAllRules();

assertThat(p.getAddress().getCity()).isEqualTo("Tokyo");
}

@Test
public void assign_deepNestedPropertyInModify() {
// DROOLS-7195
String str = "package com.example.reproducer\n" +
"import " + Person.class.getCanonicalName() + ";\n" +
"rule R\n" +
"dialect \"mvel\"\n" +
"when\n" +
" $p : Person(name == \"John\")\n" +
"then\n" +
" modify($p) {" +
" address.visitorCounter.value = 1;\n" +
" }\n" +
"end";

KieSession ksession = getKieSession(str);

Person person = new Person("John");
Address address = new Address("London");
Counter counter = new Counter();
counter.setValue(0);
address.setVisitorCounter(counter);
person.setAddress(address);
ksession.insert(person);
ksession.fireAllRules();

assertThat(person.getAddress().getVisitorCounter().getValue()).isEqualTo(1);
}

@Test
public void setter_deepNestedPropertyInModify() {
// DROOLS-7195
String str = "package com.example.reproducer\n" +
"import " + Person.class.getCanonicalName() + ";\n" +
"rule R\n" +
"dialect \"mvel\"\n" +
"when\n" +
" $p : Person(name == \"John\")\n" +
"then\n" +
" modify($p) {" +
" address.visitorCounter.setValue(1);\n" +
" }\n" +
"end";

KieSession ksession = getKieSession(str);

Person person = new Person("John");
Address address = new Address("London");
Counter counter = new Counter();
counter.setValue(0);
address.setVisitorCounter(counter);
person.setAddress(address);
ksession.insert(person);
ksession.fireAllRules();

assertThat(person.getAddress().getVisitorCounter().getValue()).isEqualTo(1);
}
}
Loading

0 comments on commit 107e91d

Please sign in to comment.