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.

Sign up for GitHub

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

[incubator-kie-issues#1543] Add the "id" of executed rules to the AfterEvaluateDecisionTableEvent #6127

Merged
merged 5 commits into from
Oct 18, 2024
Merged

[incubator-kie-issues#1543] Add the "id" of executed rules to the AfterEvaluateDecisionTableEvent #6127

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3099fe
[incubator-kie-issues#1543] Add the "id" of executed rules to the Aft…
Oct 16, 2024
d45fc7c
[incubator-kie-issues#1543] Fix TODO
Oct 16, 2024
f2a2568
[incubator-kie-issues#1543] Fix license header
Oct 17, 2024
e8d14dc
[incubator-kie-issues#1543] Minor refactoring on unrelated test
Oct 17, 2024
3e4b7e6
[incubator-kie-issues#1543] Fix as per PR review
Oct 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ...kie-dmn-api/src/main/java/org/kie/dmn/api/core/event/AfterEvaluateDecisionTableEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ default String getDecisionTableId() {
List<Integer> getMatches();

List<Integer> getSelected();

List<String> getMatchesIds();

List<String> getSelectedIds();
}
6 changes: 5 additions & 1 deletion ...ie-dmn-core-jsr223/src/main/java/org/kie/dmn/core/jsr223/JSR223DTExpressionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager dmrem, DMNResult dmnr) {
LOG.debug("failed evaluate", e);
throw new RuntimeException(e);
} finally {
DMNRuntimeEventManagerUtils.fireAfterEvaluateDecisionTable( dmrem, node.getName(), node.getName(), dt.getId(), result, (r != null ? r.matchedRules : null), (r != null ? r.fired : null) );
DMNRuntimeEventManagerUtils.fireAfterEvaluateDecisionTable( dmrem, node.getName(), node.getName(), dt.getId(), result,
(r != null ? r.matchedRules : null),
(r != null ? r.fired : null),
(r != null ? r.matchedIds : null),
(r != null ? r.firedIds : null));
}
}

Expand Down
10 changes: 9 additions & 1 deletion kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/ast/DMNDTExpressionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,11 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager dmrem, DMNResult dmnr) {
r = processEvents( events, dmrem, result, node );
return new EvaluatorResultImpl( dtr, r.hasErrors ? ResultType.FAILURE : ResultType.SUCCESS );
} finally {
DMNRuntimeEventManagerUtils.fireAfterEvaluateDecisionTable( dmrem, node.getName(), dt.getName(), dtNodeId, result, (r != null ? r.matchedRules : null), (r != null ? r.fired : null) );
DMNRuntimeEventManagerUtils.fireAfterEvaluateDecisionTable( dmrem, node.getName(), dt.getName(), dtNodeId, result,
(r != null ? r.matchedRules : null),
(r != null ? r.fired : null),
(r != null ? r.matchedIds : null),
(r != null ? r.firedIds : null));
}
}

Expand All @@ -112,8 +116,10 @@ public static EventResults processEvents(List<FEELEvent> events, DMNRuntimeEvent
for ( FEELEvent e : events ) {
if ( e instanceof DecisionTableRulesMatchedEvent ) {
r.matchedRules = ((DecisionTableRulesMatchedEvent) e).getMatches();
r.matchedIds = ((DecisionTableRulesMatchedEvent) e).getMatchesIds();
} else if ( e instanceof DecisionTableRulesSelectedEvent ) {
r.fired = ((DecisionTableRulesSelectedEvent) e).getFired();
r.firedIds = ((DecisionTableRulesSelectedEvent) e).getFiredIds();
} else if ( e.getSeverity() == FEELEvent.Severity.ERROR ) {
MsgUtil.reportMessage( logger,
DMNMessage.Severity.ERROR,
Expand Down Expand Up @@ -143,6 +149,8 @@ public static class EventResults {
public boolean hasErrors = false;
public List<Integer> matchedRules;
public List<Integer> fired;
public List<String> matchedIds;
public List<String> firedIds;
}


Expand Down
2 changes: 1 addition & 1 deletion kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNEvaluatorCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,7 +678,7 @@ protected DMNExpressionEvaluator compileDecisionTable(DMNCompilerContext ctx, DM
java.util.List<DTDecisionRule> rules = new ArrayList<>();
index = 0;
for ( DecisionRule dr : dt.getRule() ) {
DTDecisionRule rule = new DTDecisionRule( index );
DTDecisionRule rule = new DTDecisionRule( index, dr.getId() );
for ( int i = 0; i < dr.getInputEntry().size(); i++ ) {
UnaryTests ut = dr.getInputEntry().get(i);
final java.util.List<UnaryTest> tests;
Expand Down
5 changes: 4 additions & 1 deletion ...e/src/main/java/org/kie/dmn/core/compiler/alphanetbased/DMNAlphaNetworkEvaluatorImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,10 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d
} finally {
evalCtx.exitFrame();
DMNRuntimeEventManagerUtils.fireAfterEvaluateDecisionTable(eventManager, node.getName(), decisionTableName, decisionTableId, dmnResult,
(eventResults != null ? eventResults.matchedRules : null), (eventResults != null ? eventResults.fired : null));
(eventResults != null ? eventResults.matchedRules : null),
(eventResults != null ? eventResults.fired : null),
(eventResults != null ? eventResults.matchedIds : null),
(eventResults != null ? eventResults.firedIds : null));
}
}

Expand Down
46 changes: 31 additions & 15 deletions kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/alphanetbased/Results.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
Expand All @@ -29,6 +29,7 @@
import org.drools.model.functions.Function1;
import org.kie.dmn.api.feel.runtime.events.FEELEvent;
import org.kie.dmn.feel.lang.EvaluationContext;
import org.kie.dmn.feel.runtime.decisiontables.DTDecisionRule;
import org.kie.dmn.feel.runtime.decisiontables.DecisionTable;
import org.kie.dmn.feel.runtime.decisiontables.HitPolicy;
import org.kie.dmn.feel.runtime.decisiontables.Indexed;
Expand All @@ -49,16 +50,17 @@ static class Items {

public void addResult(ResultObject resultObject) {
resultGroupedByRow
.computeIfAbsent(resultObject.row, i -> new ArrayList<>(1)) // 10 (the default for Java) columns output are not that usual
.computeIfAbsent(resultObject.row, i -> new ArrayList<>(1)) // 10 (the default for Java) columns
// output are not that usual
.add(resultObject);
}

public void clearResults() {
resultGroupedByRow.clear();
}

List<Indexed> matches() {
return indexes().map(i -> (Indexed) () -> i).collect(toList());
List<DTDecisionRule> matches() {
return indexes().map(i -> new DTDecisionRule(i, null)).collect(toList());
}

private Stream<Integer> indexes() {
Expand Down Expand Up @@ -147,25 +149,39 @@ public Object applyHitPolicy(EvaluationContext evaluationContext,
}
events.add(new HitPolicyViolationEvent(
FEELEvent.Severity.WARN,
String.format("No rule matched for decision table '%s' and no default values were defined. Setting result to null.", decisionTable.getName()),
String.format("No rule matched for decision table '%s' and no default values were defined. " +
"Setting result to null.", decisionTable.getName()),
decisionTable.getName(),
Collections.emptyList()));
}

List<? extends Indexed> matchIndexes = items.matches();
evaluationContext.notifyEvt( () -> {
List<Integer> matchedIndexes = matchIndexes.stream().map( dr -> dr.getIndex() + 1 ).collect(Collectors.toList() );
return new DecisionTableRulesMatchedEvent(FEELEvent.Severity.INFO,
String.format("Rules matched for decision table '%s': %s", decisionTable.getName(), matchIndexes),
decisionTable.getName(),
decisionTable.getName(),
matchedIndexes );
}
List<DTDecisionRule> matchIndexes = items.matches();
Copy link
Contributor

Choose a reason for hiding this comment

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

When I compare this with a very similar logic in DecisionTableImpl.java I would maybe stick also here to the variable name matches index of matchIndexes as DRDecisionRule contains more data than index.

Copy link
Contributor Author

@gitgabrio gitgabrio Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

HI @jomarko I agree and see your point.
Anyway all that part needs review, i.e.

  1. verify if that alphanetbased part is actually used anywhere
  2. double check the usage of that Indexed interface (from which the name matchIndexes came originally)
  3. verify/remove all code-duplication, eventually

evaluationContext.notifyEvt(() -> {
List<Integer> matchedIndexes = new ArrayList<>();
List<String> matchesId = new ArrayList<>();
matchIndexes.forEach(dr -> {
matchedIndexes.add(dr.getIndex() + 1);
if (dr.getId() != null && !dr.getId().isEmpty()) {
matchesId.add(dr.getId());
}
});
return new DecisionTableRulesMatchedEvent(FEELEvent.Severity.INFO,
String.format("Rules matched for " +
"decision " +
"table '%s': " +
"%s",
decisionTable.getName(), matchIndexes),
decisionTable.getName(),
decisionTable.getName(),
matchedIndexes,
matchesId);
}
);

List<Object> resultObjects = items.evaluateResults(evaluationContext);

Map<Integer, String> errorMessages = checkResults(decisionTable.getOutputs(), evaluationContext, matchIndexes, resultObjects );
Map<Integer, String> errorMessages = checkResults(decisionTable.getOutputs(), evaluationContext, matchIndexes
, resultObjects);
if (!errorMessages.isEmpty()) {
List<Integer> offending = new ArrayList<>(errorMessages.keySet());
events.add(new HitPolicyViolationEvent(
Expand Down
17 changes: 15 additions & 2 deletions ...kie-dmn-core/src/main/java/org/kie/dmn/core/impl/AfterEvaluateDecisionTableEventImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,18 @@ public class AfterEvaluateDecisionTableEventImpl
private final DMNResult result;
private final List<Integer> matches;
private final List<Integer> fired;
private final List<String> matchesIds;
private final List<String> firedIds;

public AfterEvaluateDecisionTableEventImpl(String nodeName, String dtName, String dtId, DMNResult result, List<Integer> matches, List<Integer> fired) {
public AfterEvaluateDecisionTableEventImpl(String nodeName, String dtName, String dtId, DMNResult result, List<Integer> matches, List<Integer> fired, List<String> matchesIds, List<String> firedIds) {
this.nodeName = nodeName;
this.dtName = dtName;
this.dtId = dtId;
this.result = result;
this.matches = matches;
this.fired = fired;
this.matchesIds = matchesIds;
this.firedIds = firedIds;
}

@Override
Expand Down Expand Up @@ -73,9 +77,18 @@ public List<Integer> getSelected() {
return fired == null ? Collections.emptyList() : fired;
}

@Override
public List<String> getMatchesIds() {
return matchesIds == null ? Collections.emptyList() : matchesIds;
}

@Override
public List<String> getSelectedIds() {return firedIds == null ? Collections.emptyList() : firedIds;
}

@Override
public String toString() {
return "AfterEvaluateDecisionTableEvent{ nodeName='"+nodeName+"' decisionTableName='" + dtName + "' matches=" + getMatches() + " fired=" + getSelected() + " }";
return "AfterEvaluateDecisionTableEvent{ nodeName='"+nodeName+"' decisionTableName='" + dtName + "' matches=" + getMatches() + " fired=" + getSelected() + "' matchesIds=" + getMatchesIds() + " firedIds=" + getSelectedIds() + " }";
}

}
4 changes: 2 additions & 2 deletions kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNRuntimeEventManagerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ public static void fireBeforeEvaluateDecisionTable( DMNRuntimeEventManager event
}
}

public static void fireAfterEvaluateDecisionTable( DMNRuntimeEventManager eventManager, String nodeName, String dtName, String dtId, DMNResult result, List<Integer> matches, List<Integer> fired ) {
public static void fireAfterEvaluateDecisionTable( DMNRuntimeEventManager eventManager, String nodeName, String dtName, String dtId, DMNResult result, List<Integer> matches, List<Integer> fired, List<String> matchedIds, List<String> firedIds ) {
if( eventManager.hasListeners() ) {
AfterEvaluateDecisionTableEvent event = new AfterEvaluateDecisionTableEventImpl(nodeName, dtName, dtId, result, matches, fired);
AfterEvaluateDecisionTableEvent event = new AfterEvaluateDecisionTableEventImpl(nodeName, dtName, dtId, result, matches, fired, matchedIds, firedIds);
notifyListeners(eventManager, l -> l.afterEvaluateDecisionTable(event));
}
}
Expand Down
38 changes: 26 additions & 12 deletions kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNInputRuntimeTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.kie.dmn.api.core.ast.ItemDefNode;
import org.kie.dmn.api.core.event.AfterConditionalEvaluationEvent;
import org.kie.dmn.api.core.event.AfterEvaluateConditionalEvent;
import org.kie.dmn.api.core.event.AfterEvaluateDecisionTableEvent;
import org.kie.dmn.api.core.event.DMNRuntimeEventListener;
import org.kie.dmn.core.api.DMNFactory;
import org.kie.dmn.core.api.event.DefaultDMNRuntimeEventListener;
Expand Down Expand Up @@ -384,26 +385,36 @@ void typeConstraintsChecks(boolean useExecModelCompiler) {

@ParameterizedTest
@MethodSource("params")
void conditionalIfCheck(boolean useExecModelCompiler) {
void evaluationHitIdsCheck(boolean useExecModelCompiler) {
init(useExecModelCompiler);
final String ifElementId = "_3C702CE4-E5A0-4B6F-905D-C2621FFFA387";
final String thenElementId = "_6481FF12-61B5-451C-B775-4143D9B6CD6B";
final String elseElementId = "_2CD02CB2-6B56-45C4-B461-405E89D45633";
final String ruleId0 = "_1578BD9E-2BF9-4BFC-8956-1A736959C937";
final String ruleId1 = "_31CD7AA3-A806-4E7E-B512-821F82043620";
final String ruleId3 = "_2545E1A8-93D3-4C8A-A0ED-8AD8B10A58F9";
final String ruleId4 = "_510A50DA-D5A4-4F06-B0BE-7F8F2AA83740";
final DMNRuntime runtime = DMNRuntimeUtil.createRuntime("valid_models/DMNv1_5/RiskScore_Simple.dmn", this.getClass() );


final List<AfterEvaluateConditionalEvent> afterEvaluateConditionalEvents = new ArrayList<>();
final List<AfterConditionalEvaluationEvent> afterConditionalEvaluationEvents = new ArrayList<>();
final List<String> evaluateConditionalIds = new ArrayList<>();
final List<String> conditionalEvaluationIds = new ArrayList<>();
final List<String> executedRuleIds = new ArrayList<>();
runtime.addListener(new DefaultDMNRuntimeEventListener() {

@Override
public void afterConditionalEvaluation(AfterConditionalEvaluationEvent event) {
conditionalEvaluationIds.add(event.getExecutedId());
}

@Override
public void afterEvaluateConditional(AfterEvaluateConditionalEvent event) {
afterEvaluateConditionalEvents.add(event);
evaluateConditionalIds.add(event.getExecutedId());
}

@Override
public void afterConditionalEvaluation(AfterConditionalEvaluationEvent event) {
afterConditionalEvaluationEvents.add(event);
public void afterEvaluateDecisionTable(AfterEvaluateDecisionTableEvent event) {
executedRuleIds.addAll(event.getSelectedIds());
}

});
Expand All @@ -419,20 +430,23 @@ public void afterConditionalEvaluation(AfterConditionalEvaluationEvent event) {
final DMNResult dmnResult1 = runtime.evaluateAll( dmnModel, ctx1 );
assertThat(dmnResult1.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnResult1.getMessages())).isFalse();
assertThat( dmnResult1.getContext().get( "Risk Score" )).isEqualTo(BigDecimal.valueOf(50));
assertThat(afterEvaluateConditionalEvents).hasSize(1).allMatch(event -> event.getExecutedId().equals(ifElementId));
assertThat(afterConditionalEvaluationEvents).hasSize(1).allMatch(event -> event.getExecutedId().equals(elseElementId));
assertThat(evaluateConditionalIds).hasSize(1).allMatch(id -> id.equals(ifElementId));
assertThat(conditionalEvaluationIds).hasSize(1).allMatch(id -> id.equals(elseElementId));
assertThat(executedRuleIds).hasSize(2).contains(ruleId0, ruleId3);

//
afterEvaluateConditionalEvents.clear();
afterConditionalEvaluationEvents.clear();
evaluateConditionalIds.clear();
conditionalEvaluationIds.clear();
executedRuleIds.clear();
final DMNContext ctx2 = runtime.newContext();
ctx2.set("Credit Score", "Excellent");
ctx2.set("DTI", 10);
final DMNResult dmnResult2 = runtime.evaluateAll( dmnModel, ctx2 );
assertThat(dmnResult2.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnResult1.getMessages())).isFalse();
assertThat( dmnResult2.getContext().get( "Risk Score" )).isEqualTo(BigDecimal.valueOf(20));
assertThat(afterEvaluateConditionalEvents).hasSize(1).allMatch(event -> event.getExecutedId().equals(ifElementId));
assertThat(afterConditionalEvaluationEvents).hasSize(1).allMatch(event -> event.getExecutedId().equals(thenElementId));
assertThat(evaluateConditionalIds).hasSize(1).allMatch(id -> id.equals(ifElementId));
assertThat(conditionalEvaluationIds).hasSize(1).allMatch(id -> id.equals(thenElementId));
assertThat(executedRuleIds).hasSize(2).contains(ruleId1, ruleId4);
}

@ParameterizedTest
Expand Down
8 changes: 7 additions & 1 deletion ...mn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/decisiontables/DTDecisionRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,13 @@ The class DecisionRule is used to model the rules in a decision table (see 8.2 N
*/
public class DTDecisionRule implements Indexed {
private int index;
private String id;
private List<UnaryTest> inputEntry;
private List<CompiledExpression> outputEntry;

public DTDecisionRule(int index) {
public DTDecisionRule(int index, String id) {
this.index = index;
this.id = id;
}

/**
Expand Down Expand Up @@ -86,4 +88,8 @@ public List<CompiledExpression> getOutputEntry() {
public int getIndex() {
return index;
}

public String getId() {
return id;
}
}
14 changes: 11 additions & 3 deletions ...kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/decisiontables/DecisionTableImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,12 +257,20 @@ private List<DTDecisionRule> findMatches(EvaluationContext ctx, Object[] params)
}
}
ctx.notifyEvt( () -> {
List<Integer> matches = matchingDecisionRules.stream().map( dr -> dr.getIndex() + 1 ).collect( Collectors.toList() );
List<Integer> matches = new ArrayList<>();
List<String> matchesId = new ArrayList<>();
matchingDecisionRules.forEach(dr -> {
matches.add( dr.getIndex() + 1 );
if (dr.getId() != null && !dr.getId().isEmpty()) {
matchesId.add(dr.getId());
}
});
return new DecisionTableRulesMatchedEvent(FEELEvent.Severity.INFO,
"Rules matched for decision table '" + getName() + "': " + matches.toString(),
"Rules matched for decision table '" + getName() + "': " + matches,
getName(),
getName(),
matches );
matches,
matchesId);
}
);
return matchingDecisionRules;
Expand Down
Loading
Loading