[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

AthiraHari77 · 2024-12-17T06:25:19Z

In this PR, Decision models are validated to ensure correctness, and both valid and invalid decisions are identified and handled accordingly. In the event of an exception, a valid status is returned to reflect the error state accurately.

Thank you for submitting this pull request

NOTE!: Double-check the target branch for this PR.
The default is main so it will target Drools 8 / Kogito.

Ports If a forward-port or a backport is needed, paste the forward port PR here

link

Issue: (please edit the GitHub Issues link if it exists)

link

referenced Pull Requests: (please edit the URLs of referenced pullrequests if they exist)

paste the link(s) from GitHub here
link 2
link 3 etc.

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 and downstream checks
- Push a new commit to the PR. An empty commit would be enough.
for a full downstream build
- for github actions job: add the label run_fdb
for Jenkins PR check only
- If you are an ASF committer for KIE podling, login to Jenkins (https://ci-builds.apache.org/job/KIE/job/drools/), go to the specific PR job, and click on Build Now button.

…ncubator-kie-drools into incubator-kie-issues#1619

kie-dmn/kie-dmn-test-resources/src/test/resources/valid_models/DMNv1_5/MultipleDecision.dmn

yesamer · 2024-12-17T10:14:34Z

kie-dmn/kie-dmn-test-resources/src/test/resources/valid_models/DMNv1_5/MultipleDecision.dmn

@@ -0,0 +1,55 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<definitions xmlns="https://www.omg.org/spec/DMN/20230324/MODEL/" expressionLanguage="https://www.omg.org/spec/DMN/20230324/FEEL/" namespace="https://kie.org/dmn/_CADD03FC-4ABD-46D2-B631-E7FDE384D6D7" id="_9C0A9728-3B31-45F3-933F-75763212194B" name="DMN_54AA2CFA-2374-4FCE-8F16-B594DFF87EBE" xmlns:dmndi="https://www.omg.org/spec/DMN/20230324/DMNDI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" xmlns:di="http://www.omg.org/spec/DMN/20180521/DI/" xmlns:kie="https://kie.org/dmn/extensions/1.0">


@AthiraHari77 License header is still missing here

@yesamer I have updated here as well

pibizza · 2024-12-17T10:21:18Z

kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/internal/utils/DMNRuntimeBuilderTest.java

+        DMNContext context = DMNFactory.newContext();
+        context.set( "Person Age", 24 );
+        String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >= 18' for name 'Can Drive?' on node 'Can Drive?': syntax error near 'Age' (DMN id: _563E78C7-EFD1-4109-9F30-B14922EF68DF, Error compiling the referenced FEEL expression) ";
+        assertThatThrownBy(() -> dmnRuntime.evaluateByName(dmnModel, context, "Can Drive"))


I would go for the more readable assertThatIllegalStateExceptionIsThrownBy(.... Consider this also in the other tests.

@pibizza Updated the test cases as specified

pibizza

This is fine. I would probably split the test file in two - one for normal cases and one for exceptional cases. the one for exceptional cases can greatly be simplified, becoming a single parametric test. We can do this later however.

gitgabrio

Hi @AthiraHari77
great job.
But TBH I'm surprised that this modification does not break any test, apparently, and now I'm wondering if there could be something else missing.
At the same time, as maybe mentioned in private, I was pretty sure that we would need to modify, somehow, the message creation to have a better structured information.
Last, we have to be sure that our TCK implementation will be adapted accordingly, because I'm pretty sure that this modification could break the current one.

gitgabrio · 2024-12-20T07:06:38Z

kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNRuntimeImpl.java

@@ -754,6 +760,15 @@ private static String getDependencyIdentifier(DMNNode callerNode, DMNNode node)

    }

+    private static void identifyDecisionErrors(DMNModel model, String... decisions) {
+        List<DMNMessage> errorMessages = model.getMessages(DMNMessage.Severity.ERROR);
+        List<String> identifiedErrors = errorMessages.stream().map(Message::getText)


HI @AthiraHari77
Many thanks for the PR.
I'm only slightly concerned about this assertion.
The logic here is that

decisions could be decisionsId or decisionName (IINW)

if an error message contains one of them, then the decisions it relates to is "wrong"

I'm afraid there could be some cases where an error message contains the decisionId or decisionName, but that the error is not in the decision itself (e.g. another element that refer that decision)

yesamer · 2024-12-20T08:20:09Z

@gitgabrio @AthiraHari77 I ran the TCK tests against those changes, and there is only one new test failure:
1131 - feel -function invocation test: Non existing function evaluates to null (10.3.2.13.2)
I guess we should return null in case those errors occur.

athira and others added 12 commits December 1, 2024 23:22

[incubator-kie-issues#1605] Adding required model

3659d20

Merge branch 'apache:main' into incubator-kie-issues#1619

0ee347f

Merge branch 'apache:main' into incubator-kie-issues#1619

9357e98

[incubator-kie-issues#1619] Code changes

f2c9144

Merge branch 'apache:main' into incubator-kie-issues#1619

4e0d07d

Merge branch 'apache:main' into incubator-kie-issues#1619

b2cf83b

[incubator-kie-issues#1619] Code changes

e713f03

Merge branch 'incubator-kie-issues#1619' of github.com:AthiraHari77/i…

49020f4

…ncubator-kie-drools into incubator-kie-issues#1619

Merge branch 'apache:main' into incubator-kie-issues#1619

e90aac8

[incubator-kie-issues#1619] Code refactoring

b51cb93

[incubator-kie-issues#1619] Code cleanup

6310666

Merge branch 'apache:main' into incubator-kie-issues#1619

816ac31

yesamer marked this pull request as ready for review December 17, 2024 06:35

yesamer requested changes Dec 17, 2024

View reviewed changes

kie-dmn/kie-dmn-test-resources/src/test/resources/valid_models/DMNv1_5/MultipleDecision.dmn Show resolved Hide resolved

yesamer requested a review from baldimir December 17, 2024 08:53

yesamer added the DMN label Dec 17, 2024

yesamer changed the title ~~[Incubator kie issues#1619]Correctly manage execution of invalid models~~ [Incubator kie issues#1619] Correctly manage execution of invalid models Dec 17, 2024

yesamer changed the title ~~[Incubator kie issues#1619] Correctly manage execution of invalid models~~ [Incubator-kie-issues#1619] Correctly manage execution of invalid models Dec 17, 2024

[incubator-kie-issues#1619] Fix review comments

df0ef60

yesamer reviewed Dec 17, 2024

View reviewed changes

pibizza reviewed Dec 17, 2024

View reviewed changes

[incubator-kie-issues#1619] Fix review comments

2e252fe

yesamer requested a review from pibizza December 19, 2024 08:24

pibizza approved these changes Dec 19, 2024

View reviewed changes

gitgabrio requested changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

AthiraHari77 commented Dec 17, 2024 •

edited by yesamer

Loading

yesamer Dec 17, 2024

AthiraHari77 Dec 17, 2024

pibizza Dec 17, 2024

AthiraHari77 Dec 17, 2024

pibizza left a comment

gitgabrio left a comment

gitgabrio Dec 20, 2024

yesamer commented Dec 20, 2024

		@@ -0,0 +1,55 @@
		<?xml version="1.0" encoding="UTF-8" ?>
		<definitions xmlns="https://www.omg.org/spec/DMN/20230324/MODEL/" expressionLanguage="https://www.omg.org/spec/DMN/20230324/FEEL/" namespace="https://kie.org/dmn/_CADD03FC-4ABD-46D2-B631-E7FDE384D6D7" id="_9C0A9728-3B31-45F3-933F-75763212194B" name="DMN_54AA2CFA-2374-4FCE-8F16-B594DFF87EBE" xmlns:dmndi="https://www.omg.org/spec/DMN/20230324/DMNDI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" xmlns:di="http://www.omg.org/spec/DMN/20180521/DI/" xmlns:kie="https://kie.org/dmn/extensions/1.0">

[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

Are you sure you want to change the base?

[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

Conversation

AthiraHari77 commented Dec 17, 2024 • edited by yesamer Loading

yesamer Dec 17, 2024

Choose a reason for hiding this comment

AthiraHari77 Dec 17, 2024

Choose a reason for hiding this comment

pibizza Dec 17, 2024

Choose a reason for hiding this comment

AthiraHari77 Dec 17, 2024

Choose a reason for hiding this comment

pibizza left a comment

Choose a reason for hiding this comment

gitgabrio left a comment

Choose a reason for hiding this comment

gitgabrio Dec 20, 2024

Choose a reason for hiding this comment

yesamer commented Dec 20, 2024

AthiraHari77 commented Dec 17, 2024 •

edited by yesamer

Loading