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

tkobayas · 2022-12-02T10:32:32Z

…with mvel runtime (nested properties)

Ports
This is a PR for main.
for 7.x -> https://github.com/kiegroup/drools/pull/4868

JIRA:
https://issues.redhat.com/browse/DROOLS-7195

How to replicate CI configuration locally?

Build Chain tool does "simple" maven build(s), the builds are just Maven commands, but because the repositories relates and depends on each other and any change in API or class method could affect several of those repositories there is a need to use build-chain tool to handle cross repository builds and be sure that we always use latest version of the code for each repository.

build-chain tool is a build tool which can be used on command line locally or in Github Actions workflow(s), in case you need to change multiple repositories and send multiple dependent pull requests related with a change you can easily reproduce the same build by executing it on Github hosted environment or locally in your development environment. See local execution details to get more information about it.

How to retest this PR or trigger a specific build:

for pull request checks
Please add comment: Jenkins retest this
for a specific pull request check
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] tests
for a full downstream build
- for jenkins job: please add comment: Jenkins run fdb
- for github actions job: add the label run_fdb
a compile downstream build please add comment: Jenkins run cdb
a full production downstream build please add comment: Jenkins execute product fdb
an upstream build please add comment: Jenkins run upstream
for quarkus branch checks
Run checks against Quarkus current used branch
Please add comment: Jenkins run quarkus-branch
for a quarkus branch specific check
Run checks against Quarkus current used branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-branch
for quarkus main checks
Run checks against Quarkus main branch
Please add comment: Jenkins run quarkus-main
for a specific quarkus main check
Run checks against Quarkus main branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-main
for quarkus lts checks
Run checks against Quarkus lts branch
Please add comment: Jenkins run quarkus-lts
for a specific quarkus lts check
Run checks against Quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] quarkus-lts
for native checks
Run native checks
Please add comment: Jenkins run native
for a specific native check
Run native checks
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] native
for mandrel checks
Run native checks against Mandrel image
Please add comment: Jenkins run mandrel
for a specific mandrel check
Run native checks against Mandrel image
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] mandrel
for mandrel lts checks
Run native checks against Mandrel image and quarkus lts branch
Please add comment: Jenkins run mandrel-lts
for a specific mandrel lts check
Run native checks against Mandrel image and quarkus lts branch
Please add comment: Jenkins (re)run [drools|kogito-runtimes|kogito-apps|kogito-examples] mandrel-lts

kie-ci · 2022-12-02T10:41:08Z

Can one of the admins verify this patch?

kie-ci · 2022-12-02T10:41:09Z

Can one of the admins verify this patch?

kie-ci3 · 2022-12-02T12:04:32Z

(tests) - kogito-apps job #799 was: UNSTABLE
Possible explanation: This should be test failures

Please look here: https://eng-jenkins-csb-business-automation.apps.ocp-c1.prod.psi.redhat.com/job/KIE/job/kogito/job/main/job/pullrequest/job/drools.tests.downstream.kogito-apps/799/display/redirect

Test results:

PASSED: 1959
FAILED: 1

Those are the test failures:

org.kie.kogito.index.ProcessDataIndexOracleIT.testProcessInstanceEvents

java.lang.AssertionError:
1 expectation failed.
JSON path data.UserTaskInstances[0].comments.size() doesn't match.
Expected: is <1>
Actual: <0>

…with mvel runtime (nested properties)

tkobayas · 2022-12-05T07:40:05Z

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

 */
 public class PreprocessPhase {

-    private final boolean failOnEmptyRootScope;


@mariofusco I removed this failOnEmptyRootScope instance field flag, because PreprocessPhase is used by both ModifyCompiler and MvelCompiler. MvelCompiler deals with whole RHS but the flag should be effective only during parsing modify block. So I changed the flag to a method parameter boolean modify for addScopeToMethodCallExpr and assignToFieldAccess. I think this is cleaner.

tkobayas · 2022-12-05T07:44:55Z

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

+                    if (!modify) {
+                        // with(){} doesn't need to replace with FieldAccess
+                        return e;
+                    }


@mariofusco Can we drop with statement support in Drools 8? It's not written in docs (TBH, I don't know when we want to use with) and I haven't seen it in users' DRLs. If we can drop, this code could be simplified. WDYT?

Not sure if we have any test covering it, anyway yes, feel free to drop it and also do the same with an eventual test.

Thanks. I have removed related codes and tests. Also mentioned it in the next release notes.

kie-ci3 · 2022-12-05T11:50:20Z

(tests) - kogito-apps job #802 was: UNSTABLE
Possible explanation: This should be test failures

Please look here: https://eng-jenkins-csb-business-automation.apps.ocp-c1.prod.psi.redhat.com/job/KIE/job/kogito/job/main/job/pullrequest/job/drools.tests.downstream.kogito-apps/802/display/redirect

Test results:

PASSED: 1959
FAILED: 1

Those are the test failures:

Build projects / org.kie.kogito.it.jobs.ProcessAsyncIT.testAsync

java.lang.AssertionError:
1 expectation failed.
JSON path hello doesn't match.
Expected: null
Actual: Hello Tiago

tkobayas · 2022-12-05T13:35:57Z

GHA Optaplanner ubuntu Java 11: Not related to this PR

2022-12-05T10:42:35.6491088Z [ERROR] Failed to execute goal org.apache.maven.plugins:maven-assembly-plugin:3.4.2:single (package-generated-docs) on project optaplanner-docs: Failed to create assembly: Error creating assembly archive zip-with-generated-docs: Problem creating zip: Execution exception: Java heap space -> [Help 1]
2022-12-05T10:42:35.6492401Z org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.apache.maven.plugins:maven-assembly-plugin:3.4.2:single (package-generated-docs) on project optaplanner-docs: Failed to create assembly: Error creating assembly archive zip-with-generated-docs: Problem creating zip: Execution exception

kogito-apps: Flaky test

[2022-12-05T11:38:29.524Z] [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 18.247 s <<< FAILURE! - in org.kie.kogito.it.jobs.ProcessAsyncIT
[2022-12-05T11:38:29.524Z] [ERROR] org.kie.kogito.it.jobs.ProcessAsyncIT.testAsync  Time elapsed: 3.914 s  <<< FAILURE!

hellowdan

If there's a plan to backport it to 7.67.x too, I think that it would be nice to have a linked RHPAM issue so we can keep better tracking of the release changes. @tkobayas WDYT?

tkobayas · 2022-12-12T02:22:05Z

@hellowdan

If there's a plan to backport it to 7.67.x too, I think that it would be nice to have a linked RHPAM issue so we can keep better tracking of the release changes. @tkobayas WDYT?

This issue is not reported by a customer case, so I'll not backport this to 7.67.x. But I'll backport this to 7.x branch for potential future community 7.x releases.

tkobayas · 2022-12-12T02:23:18Z

@mariofusco Please merge this PR, thanks!

apache#4846) * [DROOLS-7195] Modify syntax fails when using executable model, works with mvel runtime (nested properties) * - Dropping 'with' statement support

sonarqubecloud · 2022-12-14T07:12:32Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

#4846) (#4868) * [DROOLS-7195] Modify syntax fails when using executable model, works with mvel runtime (nested properties) * - Dropping 'with' statement support

…s modify in if-block (#8) * [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

…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

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

e45ebf1

…with mvel runtime (nested properties)

tkobayas force-pushed the DROOLS-7195 branch from 0aad73e to e45ebf1 Compare December 5, 2022 05:56

tkobayas commented Dec 5, 2022

View reviewed changes

tkobayas requested review from hellowdan, mariofusco and pibizza December 5, 2022 07:45

mariofusco approved these changes Dec 5, 2022

View reviewed changes

- Dropping 'with' statement support

4a94777

hellowdan approved these changes Dec 9, 2022

View reviewed changes

mariofusco merged commit 8f1a4f5 into apache:main Dec 13, 2022

tkobayas mentioned this pull request Dec 14, 2022

[7.x] [DROOLS-7195] Modify syntax fails when using executable model, works … #4868

Merged

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

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

Uh oh!

Conversation

tkobayas commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kie-ci commented Dec 2, 2022

Uh oh!

kie-ci commented Dec 2, 2022

Uh oh!

kie-ci3 commented Dec 2, 2022

Uh oh!

tkobayas Dec 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tkobayas Dec 5, 2022

Choose a reason for hiding this comment

Uh oh!

mariofusco Dec 5, 2022

Choose a reason for hiding this comment

Uh oh!

tkobayas Dec 5, 2022

Choose a reason for hiding this comment

Uh oh!

kie-ci3 commented Dec 5, 2022

Uh oh!

tkobayas commented Dec 5, 2022

Uh oh!

hellowdan left a comment

Choose a reason for hiding this comment

Uh oh!

tkobayas commented Dec 12, 2022

Uh oh!

tkobayas commented Dec 12, 2022

Uh oh!

sonarqubecloud bot commented Dec 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tkobayas commented Dec 2, 2022 •

edited

Loading

tkobayas Dec 5, 2022 •

edited

Loading