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

DMN Runner execution identifies wrongly missing requirements #270

Closed
jomarko opened this issue May 17, 2023 · 5 comments
Closed

DMN Runner execution identifies wrongly missing requirements #270

jomarko opened this issue May 17, 2023 · 5 comments
Assignees
Labels
area:dmn Related to DMN area:tools Issues affecting Apache KIE tooling projects misc:good-first-issue Good for newcomers type:bug Something is behaving unexpectedly

Comments

@jomarko
Copy link

jomarko commented May 17, 2023

Steps to reproduce

  1. open sandbox.kie.org and setup kie extended services to be able to run DMN models
  2. import this model (remove .txt extension before importing)
    habitability.dmn.txt
  3. Open DMN tabular runner for it [1], double check the 'oxygene' and 'temperature' are empty
  4. move to problems, execution anch the messages, there is twice message about missing 'temperature', however there should be one for 'temperature' one for 'oxygene' [2]

[1]
Screenshot 2023-05-17 155947

[2]
Screenshot 2023-05-17 155902

@jomarko jomarko added type:bug Something is behaving unexpectedly area:dmn Related to DMN area:tools Issues affecting Apache KIE tooling projects labels May 17, 2023
@jomarko jomarko moved this to Backlog 💭 (unordered) in IBM and KIE Community Project - Archived! May 17, 2023
@ljmotta
Copy link

ljmotta commented May 18, 2023

I investigate it a little, the problem is in the response of the JIT Executor. Not sure why this is happening but the dmnresult route from the extended-services response has two equal messages.

@tiagobento tiagobento added the misc:good-first-issue Good for newcomers label Aug 8, 2023
@jomarko jomarko self-assigned this Aug 10, 2023
@jomarko
Copy link
Author

jomarko commented Aug 11, 2023

yes, as stated above, the issue, really seems to be in kogito-apps/jitexecutor

because if we check the typescript code in ExtendedServicesClient, slightly updated due to debugging,

  public async result(payload: ExtendedServicesModelPayload): Promise<ExtendedServicesDmnResult> {
    if (!this.isPayloadValid(payload)) {
      return { messages: [] };
    }

    const response = await fetch(this.DMN_JIT_EXECUTOR_DMN_RESULT_URL, {
      method: "POST",
      headers: {
        "Content-Type": "application/json",
        Accept: "application/json, text/plain, */*",
      },
      body: JSON.stringify(payload),
    });
    const txt =  await response.json();
    console.error("plain text to check: " + JSON.stringify(txt))
    return txt;
  }

we already see duplicated messages
image

jomarko added a commit to jomarko/drools that referenced this issue Aug 14, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 14, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 15, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 15, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 22, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 23, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 23, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`
jomarko added a commit to jomarko/drools that referenced this issue Aug 24, 2023
…rements

Closes: apache/incubator-kie-issues#270

The problem was not in the DMN Runner ui component but in the engine that executes the validation. The mechanism for storing messages includes filtering of messages. however this filtering inappropriately assumed messages bellow like equal:

- Reqiured dependency 'A' not found on node 'N'
- Required dependency 'B' not found on ndoe 'N'

```
    @OverRide
    public DMNMessage addMessage(DMNMessage newMessage) {
        for( DMNMessage existingMessage : messages ) {
            if( isDuplicate( existingMessage, newMessage ) ) {
                return existingMessage;
            }
        }
        this.messages.add( newMessage );
        return newMessage;
    }
```

`isDuplicate` didn't take into account all needed attributes of the `message`

We decided to add option to turn off this filtering of messages on backround.
As part of this PR we turn it of only for `DMNResultImpl`. We keep it unchanged on other places.
@tiagobento
Copy link

@jomarko Is this fixed? I saw there was a PR (apache/incubator-kie-drools#5461) that was merged already.. Maybe we can close this issue?

@jomarko
Copy link
Author

jomarko commented Oct 11, 2023

Hi @tiagobento , sorry for a late response, I rechecked the #270 with 0.32.0 prerelease

yes, the drools PR was merged, however the fix is not visible kie-tools, probably, because:

So think these options are possible

What is your suggestion please?

@jomarko
Copy link
Author

jomarko commented Oct 18, 2023

As explained above, the issues is not in DMN Runner itself however now in dependencies of kie-tools, we will finish the complete fix via #634

@jomarko jomarko closed this as completed Oct 18, 2023
@github-project-automation github-project-automation bot moved this from Backlog 💭 (unordered) to Done 🎉 in IBM and KIE Community Project - Archived! Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dmn Related to DMN area:tools Issues affecting Apache KIE tooling projects misc:good-first-issue Good for newcomers type:bug Something is behaving unexpectedly
Projects
None yet
Development

No branches or pull requests

3 participants