Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[KIE-792] introducing prototypes #5639

Merged
merged 10 commits into from
Jan 17, 2024
Merged

[KIE-792] introducing prototypes #5639

merged 10 commits into from
Jan 17, 2024

Conversation

mariofusco
Copy link
Contributor

@mariofusco mariofusco commented Dec 22, 2023

This is a first implementation of a KieBase option to allow compilation and evaluation of rules using prototypes as requested here: apache/incubator-kie-issues#792

For now only the simplest use case is working and of course I plan to do more work on this, even though we don't need to reach the same complexity of the complete executable model and cover all the DRL features. The main of purpose of this pull request is creating the infrastructures and abstractions necessary to accommodate the executable model generation for this new dialect.

EDIT: I did some work on this and ended up isolating prototypes in their own module and adding a new public API for them. As a consequence of this bigger refactor I changed accordingly also the Ansible integration project, see kiegroup/drools-ansible-rulebook-integration#97

Comment on lines 166 to 167
case PROTOTYPE:
rewriteConsequenceForPrototype(ruleConsequence, usedDeclarationInRHS);
case JAVA:
rewriteReassignedDeclarations(ruleConsequence, usedDeclarationInRHS);
executeCall = executeCall(ruleVariablesBlock, ruleConsequence, usedDeclarationInRHS, onCall);
Copy link
Contributor

@tkobayas tkobayas Dec 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So PROTOTYPE dialect inherits the JAVA (not MVEL) dialect RHS syntax, right? +1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must admit that for now I didn't think to much about this and I just did the quickest implementation to make that first test case to pass, but yes I think it's ok to say that the PROTOTYPE dialect is a variation of the JAVA one where we rewrite all the statement like fact.field = value as fact.set("field", value).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the accessors to map with the dot should be supported in MVEL as well but it's not currently

For Maps that use a String as a key, you may use another special syntax:

See http://mvel.documentnode.com

user.foobar
… Allowing you to treat the Map itself as a virtual object.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I knew that feature and I will give a look at how it is implemented. The problem here is that the objects inserted into the session in this case are not Map but PrototypeFact (that at the moment are backed by a Map, but this is only an internal detail), so I need to check if what we do to support mvel syntax is also a good fit for this use case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI the accessors to map with the dot should be supported in MVEL as well but it's not currently

Sorry I probably misread your comment and missed the fact that we do not support this feature in MVEL dialect even if we should. If I can make it work for prototypes I agree that it would be nice to generalize it also for MVEL maps, but I believe that it would be better to implement this with a different pull request.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So PROTOTYPE dialect inherits the JAVA (not MVEL) dialect RHS syntax, right? +1

I removed the prototype dialect and turned it into a simple compiler option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mariofusco mariofusco changed the title [KIE-792] introducing prototype dialect [KIE-792] introducing prototypes Jan 10, 2024
@kie-ci3
Copy link

kie-ci3 commented Jan 10, 2024

PR job #10 was: UNSTABLE
Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-drools/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-drools -u #5639 --skipParallelCheckout

NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/drools/job/main/job/pullrequest_jobs/job/drools-pr/job/PR-5639/10/display/redirect

Test results:

  • PASSED: 21836
  • FAILED: 1

Those are the test failures:

org.drools.quarkus.test.RuntimeTest.testDrlEvaluation java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[error]: Build step org.drools.quarkus.deployment.DroolsAssetsProcessor#generateSources threw an exception: java.lang.IllegalArgumentException: Unknown dialect: prototype
at org.drools.model.codegen.execmodel.generator.RuleContext$RuleDialect.resolveDialect(RuleContext.java:150)
at org.drools.model.codegen.execmodel.generator.RuleContext.setDialectFromAttribute(RuleContext.java:732)
at org.drools.model.codegen.execmodel.generator.ModelGenerator.processRules(ModelGenerator.java:182)
at org.drools.model.codegen.execmodel.generator.ModelGenerator.generateModel(ModelGenerator.java:158)
at org.drools.model.codegen.execmodel.processors.ModelGeneratorPhase.process(ModelGeneratorPhase.java:48)
at org.drools.compiler.builder.impl.processors.IteratingPhase.process(IteratingPhase.java:55)
at org.drools.model.codegen.tool.ExplicitCanonicalModelCompiler.process(ExplicitCanonicalModelCompiler.java:155)
at org.drools.model.codegen.project.DroolsModelBuilder.build(DroolsModelBuilder.java:123)
at org.drools.model.codegen.project.RuleCodegen.internalGenerate(RuleCodegen.java:72)
at org.drools.model.codegen.project.RuleCodegen.generate(RuleCodegen.java:106)
at org.drools.quarkus.deployment.DroolsAssetsProcessor.generateSources(DroolsAssetsProcessor.java:99)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:864)
at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
at java.base/java.lang.Thread.run(Thread.java:833)
at org.jboss.threads.JBossThread.run(JBossThread.java:501)

@mariofusco mariofusco merged commit 999a28e into apache:main Jan 17, 2024
9 checks passed
@mariofusco mariofusco deleted the kie792 branch January 17, 2024 13:29
rgdoliveira pushed a commit to rgdoliveira/drools that referenced this pull request Jan 19, 2024
* [KIE-792] introducing prototype dialect

* implement prototype field setting in consequence

* minor refactor

* rewrite prototype field access in consequence

* add dialect attribute to yaml rule format + test it in quarkus extension

* isolate prototypes in their own module

* introduce new public API for prototypes

* get rid of fact templates

* turn prototype dialect into a KieBase option

* implement access of map values via field notation in the mvel compiler
rgdoliveira pushed a commit to rgdoliveira/drools that referenced this pull request Jan 19, 2024
* [KIE-792] introducing prototype dialect

* implement prototype field setting in consequence

* minor refactor

* rewrite prototype field access in consequence

* add dialect attribute to yaml rule format + test it in quarkus extension

* isolate prototypes in their own module

* introduce new public API for prototypes

* get rid of fact templates

* turn prototype dialect into a KieBase option

* implement access of map values via field notation in the mvel compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants