KOGITO-590: Rules should "see" process variables #269

evacchi · 2019-12-23T13:07:52Z

It is now possible to declare RuleUnits implicitly. The DataSources (DataStores actually) are inferred from the process variables. e.g. if process variables are

String foo;
Person bar;

the rule unit will declare:

DataStore<String> foo;
DataStore<Person> bar;

mutation occurs in-place on the reference that is shared with the process variable.
Future work:

explicitly (possibly automatically) copy back and forth (useful especially when the rule service is a remote service)
improve codegen: right now a DataStore is always used

evacchi · 2019-12-23T13:08:50Z

jbpm/jbpm-flow-builder/src/main/java/org/jbpm/compiler/canonical/RuleSetNodeVisitor.java

+                }
+                RuleUnitComponentFactoryImpl impl = (RuleUnitComponentFactoryImpl) RuleUnitComponentFactory.get();
+                impl.registerRuleUnitDescription(generatedRuleUnitDescription);
+            }


I don't love this cast, but at the same time, I did not want to expose this in the public API. Maybe we can introduce some other interface

evacchi · 2019-12-23T13:09:40Z

api/kogito-internal/src/main/java/org/kie/internal/ruleunit/RuleUnitComponentFactory.java

@@ -30,6 +30,8 @@ static RuleUnitComponentFactory get() {

    RuleUnitDescription createRuleUnitDescription( KiePackage pkg, Class<?> ruleUnitClass );

+    RuleUnitDescription createRuleUnitDescription( KiePackage pkg, String ruleUnitSimpleName );


I guess we should backport this, although it will be a no-op

see DROOLS-4895

evacchi · 2019-12-23T13:10:45Z

drools/drools-core/src/main/java/org/drools/core/ruleunit/RuleUnitDescriptionLoader.java

+                return null;
+            } else {
+                return ruleUnitDescription;
+            }


I am not sure this change should be backported, There is no way createRuleUnitDescription(pkg, String) != null in the 7 series

see DROOLS-4895

evacchi · 2019-12-23T13:11:55Z

jbpm/jbpm-flow-builder/src/main/java/org/jbpm/compiler/canonical/RuleSetNodeVisitor.java

+                        .ifPresent(m -> m.setBody(unit(unitName)));
+//                factory.findFirst(MethodDeclaration.class, m -> m.getNameAsString().equals("unbind"))
+//                        .ifPresent(m -> m.setBody());
+            });


this is duplicated code, really. We should schedule a refactoring later KOGITO-828

(the comment is there to show that the entire snippet is duplicated)

evacchi · 2019-12-23T13:14:27Z

kogito-codegen/src/main/java/org/kie/kogito/codegen/rules/IncrementalRuleCodegen.java

-            unitKieBaseModel.setEventProcessingMode( EventProcessingOption.STREAM );
+        unitKieBaseModel.addPackage(ruleUnitDescription.getPackageName());
+
+        // fixme config should go in the description as well


these fixmes are now captured in KOGITO-827

evacchi · 2019-12-23T13:15:09Z

kogito-codegen/src/test/java/org/kie/kogito/codegen/AbstractCodegenTest.java

+                Files.write(fpath, entry.contents());
+            }
+        }
+


I have added this utility: when DEBUG mode is on, the files are also dumped to a temp directory

jbpm/jbpm-flow-builder/src/main/java/org/jbpm/compiler/canonical/RuleSetNodeVisitor.java

kogito-codegen/src/test/java/org/kie/kogito/codegen/tests/BusinessRuleUnitTest.java

mswiderski · 2020-01-07T13:15:08Z

kogito-codegen/src/test/resources/ruletask/ExampleGenerated.bpmn

+    <bpmn2:endEvent id="_B89C98A9-F577-4B07-BB72-52283EE38A77">
+      <bpmn2:incoming>_D8A8D35C-3BBE-461E-8C67-682A3CEE6ACD</bpmn2:incoming>
+    </bpmn2:endEvent>
+    <bpmn2:businessRuleTask id="_5293F325-A06A-469B-8C31-1E79EE36A3E2" drools:ruleFlowGroup="unit:ruletask.Generated" name="Rule2" implementation="http://www.jboss.org/drools/rule">


so this rule flow group value has to map to package and unit name defined in DRL or?

yes it is not actually a ruleFlowGroup that's just special syntax that's interpreted as a rule unit identifier. I chose this so that it can be retrofitted for the designer without changes. However, an alternative syntax is to just use implemenation="ruletask.Generated" and language="http://.../rule-unit" (don't remember the URI right now) but that requires a change in the designer UI

mswiderski · 2020-01-07T13:16:29Z

kogito-codegen/src/test/resources/ruletask/Generated.drl

+import org.kie.kogito.codegen.data.Person;
+
+rule singlePerson when
+    p: /singlePerson[ age >= 18 ]


can you use standard (no oopath) syntax? I tried it with just Person(age >= 18) and it didn't work...

@mswiderski
DS are named entry point so you can rewrite that OOPath expression as
p: Person( age >= 18 ) from singlePerson

thanks @danielezonca

mswiderski · 2020-01-07T13:17:04Z

kogito-codegen/src/test/resources/ruletask/Generated.drl

+rule singlePerson when
+    p: /singlePerson[ age >= 18 ]
+then
+    p.setAdult(true);


can you also add or remove data which would result in setting or removing process variables?

You can "clear" a process variable like this

singlePerson.remove(p)

if you mean declaring a new process variable or deleting... I don't think you can, but then again, you can't with processes either, as the model is now code-generated ahead of time, right? you can of course add new values to collections (e.g. collection.add(...)). The process variable declarations become the source of truth here.

(I think you can also use remove(p))

singlePerson.remove(p) does not seem to remove the value from process.

what I had in mind is that we should allow to set one process variable based on others that are evaluated as rules. So we have the person and string here so based on person we should be able to set the string - like if there is adult person then we should set "you can drink" as string.

similar we should be able to remove given value like if person is not adult we should set the string to null.

so it's not about adding new variables but more of changing values of defined variables.

oh in that case, you can refer to any variable (as long as the process declares it), but it's turned into a data source so you always have to use this dot notation (variables are always wrapped in a data source of some kind). We may refine this further with syntactic sugar, though

e.g. if you have a variable string of type String

you may (currently) write string.add("you can drink")

we could iteratively refine this by allowing "singleton" values and add syntactic sugar so that you can effectively write

string = "you can drink"

or something like that. Currently we only have "collection-like" data sources but we were already thinking about adding a "singleton" data source type

the reason why the process is not updated when you remove/add is probably because the unbind part of the rule node is not implemented yet (the lines that were commented away), so that's why some changes are not reflected. That piece of code gets called when control is returned from the rule engine to the process engine

ok, made some implementation to provide support for it, sent PR to your branch evacchi#1 please have a look and see if it is of any use

…unit's data sources (#1)

sonarqubecloud · 2020-01-13T14:16:39Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
13 Code Smells

0.0% Coverage
0.0% Duplication

evacchi added 4 commits December 23, 2019 10:44

KOGITO-590: Rules should "see" process variables

f2b7db0

add test case

73839b2

fix

65b84a0

rm comments

c6c3b47

evacchi commented Dec 23, 2019

View reviewed changes

evacchi requested review from mswiderski and mariofusco and removed request for mswiderski December 23, 2019 13:18

fix npe in model tests

7f38e01

mariofusco approved these changes Jan 2, 2020

View reviewed changes

Merge branch 'master' into K590

7bbd10f

mswiderski reviewed Jan 7, 2020

View reviewed changes

evacchi and others added 3 commits January 8, 2020 14:15

address comments, cleanup imports

8580599

disable test

d6244bf

add support for additional operations on process variables from rule …

991bc7b

…unit's data sources (#1)

mswiderski merged commit bb232bd into apache:master Jan 13, 2020

kie-ci1 mentioned this pull request Jun 1, 2022

KOGITO-7317 - Align Data Index image with runtime version #2235

Merged

kie-ci3 mentioned this pull request Apr 5, 2023

KOGITO-7900: GHA sonar integration #2474

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KOGITO-590: Rules should "see" process variables #269

KOGITO-590: Rules should "see" process variables #269

evacchi commented Dec 23, 2019 •

edited

Loading

evacchi Dec 23, 2019

evacchi Dec 23, 2019

evacchi Dec 24, 2019

evacchi Dec 23, 2019

evacchi Dec 24, 2019

evacchi Dec 23, 2019 •

edited

Loading

evacchi Dec 24, 2019 •

edited

Loading

evacchi Dec 23, 2019

evacchi Dec 23, 2019

mswiderski Jan 7, 2020

evacchi Jan 8, 2020

mswiderski Jan 7, 2020

danielezonca Jan 7, 2020

mswiderski Jan 7, 2020

mswiderski Jan 7, 2020

evacchi Jan 8, 2020 •

edited

Loading

evacchi Jan 8, 2020

mswiderski Jan 8, 2020

evacchi Jan 9, 2020 •

edited

Loading

evacchi Jan 9, 2020 •

edited

Loading

mswiderski Jan 9, 2020

sonarqubecloud bot commented Jan 13, 2020

		@@ -30,6 +30,8 @@ static RuleUnitComponentFactory get() {

		RuleUnitDescription createRuleUnitDescription( KiePackage pkg, Class<?> ruleUnitClass );

		RuleUnitDescription createRuleUnitDescription( KiePackage pkg, String ruleUnitSimpleName );

KOGITO-590: Rules should "see" process variables #269

KOGITO-590: Rules should "see" process variables #269

Conversation

evacchi commented Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evacchi Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

evacchi Dec 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evacchi Jan 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

evacchi Jan 9, 2020 • edited Loading

Choose a reason for hiding this comment

evacchi Jan 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 13, 2020

evacchi commented Dec 23, 2019 •

edited

Loading

evacchi Dec 23, 2019 •

edited

Loading

evacchi Dec 24, 2019 •

edited

Loading

evacchi Jan 8, 2020 •

edited

Loading

evacchi Jan 9, 2020 •

edited

Loading

evacchi Jan 9, 2020 •

edited

Loading