Skip to content

Commit

Permalink
Fix DecisionTable no rule match and no default means null (apache#26)
Browse files Browse the repository at this point in the history 
* Fix FEEL DecisionTable no rule match and no default, return Either for null

* Fix DMN DecisionTable no rule match and no default test case and Event

* Empty DT is WARN, and proper InvalidInputEvent handling
  • Loading branch information
@tarilabs @etirelli
tarilabs authored and etirelli committed Dec 22, 2016
1 parent 9ab3f8c commit bcf2fee
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 45 deletions.
2 changes: 2 additions & 0 deletions kie-dmn-core/src/main/java/org/kie/dmn/core/ast/DMNDTExpressionEvaluator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ private EventResults processEvents(List<FEELEvent> events, InternalDMNRuntimeEve
} else if ( e.getSeverity() == FEELEvent.Severity.ERROR ) {
result.addMessage( DMNMessage.Severity.ERROR, e.getMessage(), node.getId(), e );
r.hasErrors = true;
} else if ( e.getSeverity() == FEELEvent.Severity.WARN ) {
result.addMessage( DMNMessage.Severity.WARN, e.getMessage(), node.getId(), e );
}
}
events.clear();
Expand Down
27 changes: 27 additions & 0 deletions kie-dmn-core/src/test/java/org/kie/dmn/core/DMNDecisionTableRuntimeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,17 @@
import org.junit.Test;
import org.kie.dmn.core.api.DMNContext;
import org.kie.dmn.core.api.DMNFactory;
import org.kie.dmn.core.api.DMNMessage;
import org.kie.dmn.core.api.DMNModel;
import org.kie.dmn.core.api.DMNResult;
import org.kie.dmn.core.api.DMNRuntime;
import org.kie.dmn.core.api.event.AfterEvaluateDecisionTableEvent;
import org.kie.dmn.core.api.event.DMNEvent;
import org.kie.dmn.core.api.event.DMNRuntimeEventListener;
import org.kie.dmn.core.util.DMNRuntimeUtil;
import org.kie.dmn.feel.runtime.events.FEELEvent;
import org.kie.dmn.feel.runtime.events.HitPolicyViolationEvent;
import org.kie.dmn.feel.runtime.events.FEELEvent.Severity;
import org.mockito.ArgumentCaptor;
import org.mockito.Mockito;

Expand Down Expand Up @@ -80,6 +85,28 @@ public void testSimpleDecisionTableUniqueHitPolicySatisfies() {
assertThat( result.get( "Approval Status" ), nullValue() );
assertTrue( dmnResult.getMessages().size() > 0 );
}

@Test
public void testSimpleDecisionTableUniqueHitPolicyNullWarn() {
DMNRuntime runtime = DMNRuntimeUtil.createRuntime( "0004-simpletable-U-noinputvalues.dmn", this.getClass() );
DMNModel dmnModel = runtime.getModel( "https://github.com/droolsjbpm/kie-dmn", "0004-simpletable-U-noinputvalues" );
assertThat( dmnModel, notNullValue() );

DMNContext context = DMNFactory.newContext();
context.set( "Age", new BigDecimal( 18 ) );
context.set( "RiskCategory", "ASD" );
context.set( "isAffordable", false );

DMNResult dmnResult = runtime.evaluateAll( dmnModel, context );

DMNContext result = dmnResult.getContext();

assertThat( result.get( "Approval Status" ), nullValue() );
assertTrue( dmnResult.getMessages().size() > 0 );
assertTrue( dmnResult.getMessages().stream().anyMatch( dm -> dm.getSeverity().equals(DMNMessage.Severity.WARN)
&& dm.getFeelEvent() instanceof HitPolicyViolationEvent
&& dm.getFeelEvent().getSeverity().equals(FEELEvent.Severity.WARN) ) );
}

@Test
public void testDecisionTableWithCalculatedResult() {
Expand Down
124 changes: 124 additions & 0 deletions kie-dmn-core/src/test/resources/org/kie/dmn/core/0004-simpletable-U-noinputvalues.dmn
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
~ Licensed under the Apache License, Version 2.0 (the "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
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<definitions xmlns="http://www.omg.org/spec/DMN/20151101/dmn.xsd"
xmlns:feel="http://www.omg.org/spec/FEEL/20140401"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
id="_edbd2d8e-a5a8-4660-9bb9-adaa792d900c"
namespace="https://github.com/droolsjbpm/kie-dmn"
name="0004-simpletable-U-noinputvalues"
xsi:schemaLocation="http://www.omg.org/spec/DMN/20151101/dmn.xsd"
expressionLanguage="http://www.omg.org/spec/FEEL/20140401"
typeLanguage="http://www.omg.org/spec/FEEL/20140401">
<decision id="_0004-simpletable-U-noinputvalues" name="0004-simpletable-U-noinputvalues">
<variable name="Approval Status" typeRef="feel:string"/>
<informationRequirement>
<requiredInput href="#_Age"/>
</informationRequirement>
<informationRequirement>
<requiredInput href="#_RiskCategory"/>
</informationRequirement>
<informationRequirement>
<requiredInput href="#_isAffordable"/>
</informationRequirement>
<decisionTable hitPolicy="UNIQUE" outputLabel="Approval Status" preferredOrientation="Rule-as-Row">
<input id="_iAge" label="Age">
<inputExpression typeRef="feel:number">
<text>Age</text>
</inputExpression>
</input>
<input id="_iRiskCategory" label="RiskCategory">
<inputExpression typeRef="feel:string">
<text>RiskCategory</text>
</inputExpression>
</input>
<input id="_iIsAffordable" label="isAffordable">
<inputExpression typeRef="feel:boolean">
<text>isAffordable</text>
</inputExpression>
</input>
<output id="_oApprovalStatus">
<outputValues>
<text>"Approved", "Declined"</text>
</outputValues>
</output>
<rule id="_7f03803d-2636-40ab-8346-7fd7f38ab695">
<inputEntry id="_7f03803d-2636-40ab-8346-7fd7f38ab695-0">
<text>&gt;=18</text>
</inputEntry>
<inputEntry id="_7f03803d-2636-40ab-8346-7fd7f38ab695-1">
<text>"Medium","Low"</text>
</inputEntry>
<inputEntry id="_7f03803d-2636-40ab-8346-7fd7f38ab695-2">
<text>true</text>
</inputEntry>
<outputEntry id="_7f03803d-2636-40ab-8346-7fd7f38ab695-3">
<text>"Approved"</text>
</outputEntry>
</rule>
<rule id="_887acecd-40fc-42da-9443-eeba476f5516">
<inputEntry id="_887acecd-40fc-42da-9443-eeba476f5516-0">
<text>&lt;18</text>
</inputEntry>
<inputEntry id="_887acecd-40fc-42da-9443-eeba476f5516-1">
<text>"Medium","Low"</text>
</inputEntry>
<inputEntry id="_887acecd-40fc-42da-9443-eeba476f5516-2">
<text>true</text>
</inputEntry>
<outputEntry id="_887acecd-40fc-42da-9443-eeba476f5516-3">
<text>"Declined"</text>
</outputEntry>
</rule>
<rule id="_18058414-a571-4375-991f-77b9ea7fc699">
<inputEntry id="_18058414-a571-4375-991f-77b9ea7fc699-0">
<text>-</text>
</inputEntry>
<inputEntry id="_18058414-a571-4375-991f-77b9ea7fc699-1">
<text>"High"</text>
</inputEntry>
<inputEntry id="_18058414-a571-4375-991f-77b9ea7fc699-2">
<text>true</text>
</inputEntry>
<outputEntry id="_18058414-a571-4375-991f-77b9ea7fc699-3">
<text>"Declined"</text>
</outputEntry>
</rule>
<rule id="_ede3e62a-43f3-49d3-90a4-ffaf1f698f54">
<inputEntry id="_ede3e62a-43f3-49d3-90a4-ffaf1f698f54-0">
<text>-</text>
</inputEntry>
<inputEntry id="_ede3e62a-43f3-49d3-90a4-ffaf1f698f54-1">
<text>"High"</text>
</inputEntry>
<inputEntry id="_ede3e62a-43f3-49d3-90a4-ffaf1f698f54-2">
<text>false</text>
</inputEntry>
<outputEntry id="_ede3e62a-43f3-49d3-90a4-ffaf1f698f54-3">
<text>"Declined"</text>
</outputEntry>
</rule>
</decisionTable>
</decision>
<inputData id="_Age" name="Age">
<variable name="Age" typeRef="feel:number"/>
</inputData>
<inputData id="_RiskCategory" name="RiskCategory">
<variable name="RiskCategory" typeRef="feel:string"/>
</inputData>
<inputData id="_isAffordable" name="isAffordable">
<variable name="isAffordable" typeRef="feel:boolean"/>
</inputData>
</definitions>
79 changes: 38 additions & 41 deletions kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/decisiontables/DecisionTableImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,20 @@
import org.kie.dmn.feel.runtime.UnaryTest;
import org.kie.dmn.feel.runtime.events.DecisionTableRulesMatchedEvent;
import org.kie.dmn.feel.runtime.events.FEELEvent;
import org.kie.dmn.feel.runtime.events.FEELEventBase;
import org.kie.dmn.feel.runtime.events.HitPolicyViolationEvent;
import org.kie.dmn.feel.runtime.events.InvalidInputEvent;
import org.kie.dmn.feel.runtime.events.FEELEvent.Severity;
import org.kie.dmn.feel.runtime.functions.FEELFnResult;
import org.kie.dmn.feel.util.Either;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

Expand Down Expand Up @@ -70,42 +75,37 @@ public DecisionTableImpl(String name,
* decision table that are expressions derived from these parameters
* @return
*/
public Object evaluate(EvaluationContext ctx, Object[] params) {
public FEELFnResult<Object> evaluate(EvaluationContext ctx, Object[] params) {
if ( decisionRules.isEmpty() ) {
return null;
return FEELFnResult.ofError(new FEELEventBase(Severity.WARN, "Decision table is empty", null));
}

FEEL feel = FEEL.newInstance();
Object[] actualInputs = resolveActualInputs( ctx, feel );

try {
FEEL feel = FEEL.newInstance();
Object[] actualInputs = resolveActualInputs( ctx, feel );

if ( ! actualInputsMatchInputValues( ctx, actualInputs ) ) {
return null;
}
Either<FEELEvent, Object> actualInputMatch = actualInputsMatchInputValues( ctx, actualInputs );
if ( actualInputMatch.isLeft() ) {
return actualInputMatch.cata( e -> FEELFnResult.ofError(e), e -> FEELFnResult.ofError(null) );
}

List<DTDecisionRule> matches = findMatches( ctx, actualInputs );
if( !matches.isEmpty() ) {
List<Object> results = evaluateResults( ctx, feel, actualInputs, matches );
Object result = hitPolicy.getDti().dti( ctx, this, actualInputs, matches, results );
List<DTDecisionRule> matches = findMatches( ctx, actualInputs );
if( !matches.isEmpty() ) {
List<Object> results = evaluateResults( ctx, feel, actualInputs, matches );
Object result = hitPolicy.getDti().dti( ctx, this, actualInputs, matches, results );

return result;
return FEELFnResult.ofResult( result );
} else {
// check if there is a default value set for the outputs
if( hasDefaultValues ) {
Object result = defaultToOutput( ctx, feel );
return FEELFnResult.ofResult( result );
} else {
// check if there is a default value set for the outputs
if( hasDefaultValues ) {
Object result = defaultToOutput( ctx, feel );
return result;
} else {
FEELEventListenersManager.notifyListeners(ctx.getEventsManager(), () -> new HitPolicyViolationEvent(
FEELEvent.Severity.WARN,
"No rule matched for decision table '" + name + "' and no default values were defined. Setting result to null.",
name,
Collections.EMPTY_LIST ) );
}
return null;
return FEELFnResult.ofError( new HitPolicyViolationEvent(
Severity.WARN,
"No rule matched for decision table '" + name + "' and no default values were defined. Setting result to null.",
name,
Collections.EMPTY_LIST ) );
}
} catch ( Throwable e ) {
// no longer needed as DTInvokerFunction support Either for wrapping the exception : logger.error( "Error invoking decision table '" + getName() + "'.", e );
throw e;
}
}

Expand All @@ -124,7 +124,7 @@ private Object[] resolveActualInputs(EvaluationContext ctx, FEEL feel) {
* @param params
* @return
*/
private boolean actualInputsMatchInputValues(EvaluationContext ctx, Object[] params) {
private Either<FEELEvent, Object> actualInputsMatchInputValues(EvaluationContext ctx, Object[] params) {
// check that all the parameters match the input list values if they are defined
for( int i = 0; i < params.length; i++ ) {
final DTInputClause input = inputs.get( i );
Expand All @@ -134,20 +134,17 @@ private boolean actualInputsMatchInputValues(EvaluationContext ctx, Object[] par
boolean satisfies = input.getInputValues().stream().map( ut -> ut.apply( ctx, parameter ) ).filter( Boolean::booleanValue ).findAny().orElse( false );

if ( !satisfies ) {
FEELEventListenersManager.notifyListeners( ctx.getEventsManager(), () -> {
String values = input.getInputValuesText();
return new InvalidInputEvent( FEELEvent.Severity.ERROR,
input.getInputExpression()+"='" + parameter + "' does not match any of the valid values " + values + " for decision table '" + getName() + "'.",
getName(),
null,
values );
}
);
return false;
String values = input.getInputValuesText();
return Either.ofLeft(new InvalidInputEvent( FEELEvent.Severity.ERROR,
input.getInputExpression()+"='" + parameter + "' does not match any of the valid values " + values + " for decision table '" + getName() + "'.",
getName(),
null,
values )
);
}
}
}
return true;
return Either.ofRight(true);
}

/**
Expand Down
3 changes: 1 addition & 2 deletions kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/functions/DTInvokerFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ public FEELFnResult<Object> invoke(EvaluationContext ctx, Object[] params) {
for( int i = 0; i < params.length; i++ ) {
ctx.setValue( dt.getParameterNames().get( i ), params[i] );
}
Object result = dt.evaluate( ctx, params );
return FEELFnResult.ofResult( result );
return dt.evaluate( ctx, params );
} catch ( Exception e ) {
capturedException = new FEELEventBase(Severity.ERROR, "Error invoking decision table '"+getName()+"'.", e);
} finally {
Expand Down
17 changes: 16 additions & 1 deletion kie-dmn-feel/src/test/java/org/kie/dmn/feel/lang/examples/SimpleDecisionTablesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,14 @@
import org.junit.Test;
import org.kie.dmn.feel.FEEL;
import org.kie.dmn.feel.runtime.events.FEELEvent;
import org.kie.dmn.feel.runtime.events.FEELEvent.Severity;
import org.kie.dmn.feel.runtime.events.FEELEventListener;
import org.kie.dmn.feel.runtime.events.HitPolicyViolationEvent;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.List;
Expand Down Expand Up @@ -44,12 +48,18 @@ public static void setupTest() {
}
logger.error(" [stacktraces omitted.]");
}
}
} else if (evt.getSeverity() == FEELEvent.Severity.WARN) {
logger.warn("{}", evt);
}
} );
}

@Test
public void testMain() {
List<FEELEvent> events = new ArrayList<>();
FEELEventListener listener = evt -> events.add(evt);
feel.addListener(listener);

String expression = loadExpression( "simple_decision_tables.feel" );
Map context = (Map) feel.evaluate( expression );

Expand All @@ -63,6 +73,11 @@ public void testMain() {
assertThat( (Map<?, ?>) context.get( "result4" ), hasSize(2));
assertThat( (Map<?, ?>) context.get( "result4" ), hasEntry("Out1", "io1a" ));
assertThat( (Map<?, ?>) context.get( "result4" ), hasEntry("Out2", "io2a" ));
assertThat( context.get( "result5" ), nullValue() );
assertTrue( events.stream().anyMatch(e -> e instanceof HitPolicyViolationEvent
&& e.getSeverity().equals(Severity.WARN) ) );

feel.removeListener(listener);
}

@Test
Expand Down
13 changes: 12 additions & 1 deletion kie-dmn-feel/src/test/resources/org/kie/dmn/feel/lang/examples/simple_decision_tables.feel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,20 @@
],
hit policy: "U"
),

checkNull : decision table (
outputs: "Adult",
input expression list: ["Age"],
rule list: [
[ <18, "\"Young\""],
[ [>18, <99], "\"Adult\""]
],
hit policy: "U"
),

result1: myDecisionTable( 34 ),
result2: biggerDecisionTable( 35, "good" ),
result3: multipleOutputs(2),
result4: multipleInOut(In2: 47, In1:2)
result4: multipleInOut(In2: 47, In1:2),
result5: checkNull(9999)
}

0 comments on commit bcf2fee

Please sign in to comment.