[DROOLS-7195] Modify syntax fails when using executable model, works …

…… (#4846) (#4868)

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

* - Dropping 'with' statement support

drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/CompilationFailuresTest.java

@@ -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

drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/MvelDialectTest.java

@@ -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;
 
@@ -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);
+    }
 }

drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/domain/Address.java

@@ -6,6 +6,7 @@ public class Address {
     private int number;
     private short shortNumber;
     private String city;
+    private Counter visitorCounter = new Counter();
 
     public Address() {
         this("", 0, "");
@@ -58,6 +59,14 @@ public void setShortNumber(short shortNumber) {
         this.shortNumber = shortNumber;
     }
 
+    public Counter getVisitorCounter() {
+        return visitorCounter;
+    }
+
+    public void setVisitorCounter(Counter visitorCounter) {
+        this.visitorCounter = visitorCounter;
+    }
+
     public int hashCode() {
         final int prime = 31;
         int result = 1;

drools-model/drools-model-compiler/src/test/java/org/drools/modelcompiler/domain/Counter.java

@@ -0,0 +1,29 @@
+/*
+ * Copyright 2022 Red Hat, Inc. and/or its affiliates.
+ *
+ * Licensed 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
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.modelcompiler.domain;
+
+
+public class Counter {
+    private int value;
+
+    public int getValue() {
+        return value;
+    }
+
+    public void setValue(int value) {
+        this.value = value;
+    }
+}

drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/LHSPhase.java

@@ -126,7 +126,11 @@ public TypedExpression visit(FieldAccessExpr n, Void arg) {
         TypedExpression fieldAccessScope = n.getScope().accept(this, arg);
         n.getName().accept(this, arg);
 
-        if(parentIsArrayAccessExpr(n)) {
+        if (n.isInternal()) {
+            // a part of a larger FieldAccessExpr. e.g. [$p.address] of [$p.address.city]
+            return tryParseItAsGetter(n, fieldAccessScope)
+                    .orElse(new UnalteredTypedExpression(n));
+        } else if(parentIsArrayAccessExpr(n)) {
             return tryParseItAsMap(n, fieldAccessScope)
                     .map(Optional::of)
                     .orElseGet(() -> tryParseItAsSetter(n, fieldAccessScope, getRHSType()))
@@ -262,6 +266,16 @@ private Optional<TypedExpression> tryParseItAsSetter(FieldAccessExpr n, TypedExp
         });
     }
 
+    private Optional<TypedExpression> tryParseItAsGetter(FieldAccessExpr n, TypedExpression scope) {
+        return scope.getType().flatMap(scopeType -> {
+            String propertyName = printNode(n.getName());
+            Optional<Method> optAccessor =
+                    ofNullable(getAccessor((Class<?>) scopeType, propertyName));
+
+            return optAccessor.map(accessor -> new FieldToAccessorTExpr(scope, accessor, emptyList()));
+        });
+    }
+
     @Override
     public TypedExpression visit(MethodCallExpr n, Void arg) {
         logPhase("MethodCallExpr {}", n);

drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/ModifyCompiler.java

@@ -32,7 +32,7 @@
 // A special case of compiler in which only the modify statements are processed
 public class ModifyCompiler {
 
-    private static final PreprocessPhase preprocessPhase = new PreprocessPhase(true);
+    private static final PreprocessPhase preprocessPhase = new PreprocessPhase();
 
     public CompiledBlockResult compile(String mvelBlock) {
 

drools-model/drools-mvel-compiler/src/main/java/org/drools/mvelcompiler/MvelCompiler.java

@@ -26,7 +26,6 @@
 import com.github.javaparser.ast.stmt.Statement;
 import org.drools.mvel.parser.MvelParser;
 import org.drools.mvel.parser.ast.expr.ModifyStatement;
-import org.drools.mvel.parser.ast.expr.WithStatement;
 import org.drools.mvelcompiler.ast.TypedExpression;
 import org.drools.mvelcompiler.context.MvelCompilerContext;
 
@@ -56,13 +55,7 @@ public CompiledBlockResult compileStatement(String mvelBlock) {
                 .flatMap(this::transformStatementWithPreprocessing)
                 .collect(toList());
 
-        List<String> withUsedBindings = mvelExpression.findAll(WithStatement.class)
-                .stream()
-                .flatMap(this::transformStatementWithPreprocessing)
-                .collect(toList());
-
         allUsedBindings.addAll(modifyUsedBindings);
-        allUsedBindings.addAll(withUsedBindings);
 
         // Entry point of the compiler
         TypedExpression compiledRoot = mvelExpression.accept(statementVisitor, null);