Drools 1535 - improving error reporting for unknown variables (apache…

…#1237) * DROOLS-1535: adding compilation validation to DT input expressions * DROOLS-1535: Improving compilation to report unknown variables for decision table input expressions * DROOLS-1535: Improving compilation to report unknown variables for decision table input expressions * Drools 1535+tarilabs (#1) * Compile DT output to provide the users with compile time feedback on errors.
winklerm · May 7, 2017 · 6bab04f · 6bab04f
1 parent 055b842
commit 6bab04f
Show file tree

Hide file tree

Showing 19 changed files with 2,580 additions and 46 deletions.
diff --git a/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/feel/runtime/events/FEELEventListener.java b/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/feel/runtime/events/FEELEventListener.java
@@ -19,6 +19,7 @@
 /**
  * A general interface for a FEEL event listener
  */
+@FunctionalInterface
 public interface FEELEventListener {
 
     void onEvent( FEELEvent event );

diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNEvaluatorCompiler.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNEvaluatorCompiler.java
@@ -239,6 +239,7 @@ private DMNExpressionEvaluator compileDecisionTable(DMNCompilerContext ctx, DMNM
         java.util.List<DTInputClause> inputs = new ArrayList<>();
         int index = 0;
         for ( InputClause ic : dt.getInput() ) {
+            index++;
             String inputExpressionText = ic.getInputExpression().getText();
             String inputValuesText =  Optional.ofNullable( ic.getInputValues() ).map( UnaryTests::getText).orElse( null);
             java.util.List<UnaryTest> inputValues = null;
@@ -250,13 +251,22 @@ private DMNExpressionEvaluator compileDecisionTable(DMNCompilerContext ctx, DMNM
                                                    Msg.ERR_COMPILING_FEEL_EXPR_ON_DT_INPUT_CLAUSE_IDX,
                                                    inputValuesText,
                                                    node.getIdentifierString(),
-                                                   ++index );
+                                                   index );
             } else if ( ic.getInputExpression().getTypeRef() != null ) {
                 QName inputExpressionTypeRef = ic.getInputExpression().getTypeRef();
                 BaseDMNTypeImpl typeRef = (BaseDMNTypeImpl) model.getTypeRegistry().resolveType(resolveNamespaceForTypeRef(inputExpressionTypeRef, ic.getInputExpression()), inputExpressionTypeRef.getLocalPart());
                 inputValues = typeRef.getAllowedValues();
             }
-            inputs.add( new DTInputClause(inputExpressionText, inputValuesText, inputValues) );
+            CompiledExpression compiledInput = feel.compileFeelExpression(
+                    ctx,
+                    inputExpressionText,
+                    model,
+                    dt,
+                    Msg.ERR_COMPILING_FEEL_EXPR_ON_DT_INPUT_CLAUSE_IDX,
+                    inputExpressionText,
+                    dtName,
+                    index );
+            inputs.add( new DTInputClause(inputExpressionText, inputValuesText, inputValues, compiledInput ) );
         }
         java.util.List<DTOutputClause> outputs = new ArrayList<>();
         index = 0;
@@ -334,8 +344,17 @@ private DMNExpressionEvaluator compileDecisionTable(DMNCompilerContext ctx, DMNM
                 } ) );
             }
             for ( LiteralExpression le : dr.getOutputEntry() ) {
-                // we might want to compile and save the compiled expression here
-                rule.getOutputEntry().add( le.getText() );
+                String expressionText = le.getText();
+                CompiledExpression compiledExpression = feel.compileFeelExpression(
+                        ctx,
+                        expressionText,
+                        model,
+                        dr,
+                        Msg.ERR_COMPILING_FEEL_EXPR_ON_DT_RULE_IDX,
+                        expressionText,
+                        dtName,
+                        index+1 );
+                rule.getOutputEntry().add( compiledExpression );
             }
             rules.add( rule );
             index++;

diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNFEELHelper.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNFEELHelper.java
@@ -94,8 +94,7 @@ public List<UnaryTest> evaluateUnaryTests(DMNCompilerContext ctx, String unaryTe
         try {
             Map<String, Type> variableTypes = new HashMap<>();
             for ( Map.Entry<String, DMNType> entry : ctx.getVariables().entrySet() ) {
-                // TODO: need to properly resolve types here
-                variableTypes.put( entry.getKey(), BuiltInType.UNKNOWN );
+                variableTypes.put( entry.getKey(), ((BaseDMNTypeImpl) entry.getValue()).getFeelType() );
             }
             result = feel.evaluateUnaryTests( unaryTests, variableTypes );
         } catch( Throwable t ) {

diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/Msg.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/Msg.java
@@ -58,7 +58,7 @@ public class Msg {
     public static final Message3 ERROR_EVAL_NODE_DEP_WRONG_TYPE                      = new Message3( DMNMessageType.ERROR_EVAL_NODE, "Error while evaluating node '%s' for dependency '%s': the dependency value '%s' is not allowed by the declared type" );
     public static final Message3 ERROR_EVAL_NODE_RESULT_WRONG_TYPE                   = new Message3( DMNMessageType.ERROR_EVAL_NODE, "Error while evaluating node '%s': the declared result type is '%s' but the actual value '%s' is not an instance of that type" );
     public static final Message2 EXPR_TYPE_NOT_SUPPORTED_IN_NODE                     = new Message2( DMNMessageType.EXPR_TYPE_NOT_SUPPORTED_IN_NODE, "Expression type '%s' not supported in node '%s'" );
-    public static final Message3 ERR_COMPILING_FEEL_EXPR_ON_DT_INPUT_CLAUSE_IDX      = new Message3( DMNMessageType.ERR_COMPILING_FEEL, "Error compiling FEEL expression '%s' on decision table '%s', input clause #%s" );
+    public static final Message4 ERR_COMPILING_FEEL_EXPR_ON_DT_INPUT_CLAUSE_IDX      = new Message4( DMNMessageType.ERR_COMPILING_FEEL, "Error compiling FEEL expression '%s' on decision table '%s', input clause #%s: %s" );
     public static final Message3 ERR_COMPILING_FEEL_EXPR_ON_DT_OUTPUT_CLAUSE_IDX     = new Message3( DMNMessageType.ERR_COMPILING_FEEL, "Error compiling FEEL expression '%s' on decision table '%s', output clause #%s" );
     public static final Message3 ERR_COMPILING_FEEL_EXPR_ON_DT_RULE_IDX              = new Message3( DMNMessageType.ERR_COMPILING_FEEL, "Error compiling FEEL expression '%s' on decision table '%s', rule #%s" );
     public static final Message2 ERR_COMPILING_FEEL_EXPR_ON_DT                       = new Message2( DMNMessageType.ERR_COMPILING_FEEL, "Error compiling FEEL expression '%s' on decision table '%s'" );

diff --git a/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java b/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java
@@ -46,6 +46,7 @@
 import static org.hamcrest.Matchers.hasEntry;
 import static org.hamcrest.Matchers.hasSize;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
@@ -434,20 +435,10 @@ public void testRelation() {
     @Test
     public void testMissingInputData() {
         DMNRuntime runtime = DMNRuntimeUtil.createRuntime( "missing_input_data.dmn", getClass() );
-//        runtime.addListener( DMNRuntimeUtil.createListener() );
-
         DMNModel dmnModel = runtime.getModel( "http://www.trisotech.com/definitions/_4047acf3-fce2-42f3-abf2-fb06282c1ea0", "Upgrade Based On Promotions" );
         assertThat( dmnModel, notNullValue() );
-        assertThat( formatMessages( dmnModel.getMessages() ), dmnModel.hasErrors(), is( false ) );
-
-        DMNContext context = DMNFactory.newContext();
-        context.set( "Requested Vehicle Class", "Compact" );
-        context.set( "Membership Level", "Silver" );
-        context.set( "Calendar Promotion", "None" );
-        DMNResult dmnResult = runtime.evaluateAll( dmnModel, context );
-        //        System.out.println(formatMessages( dmnResult.getMessages() ));
-        // work in progress... later we will check the actual messages...
-        //        assertThat( formatMessages( dmnResult.getMessages() ), dmnResult.getMessages( DMNMessage.Severity.ERROR ).size(), is( 4 ) );
+        assertThat( formatMessages( dmnModel.getMessages() ), dmnModel.hasErrors(), is( true ) );
+        assertThat( dmnModel.getMessages().get( 0 ).getMessageType(), is( DMNMessageType.ERR_COMPILING_FEEL ) );
     }
 
     @Test
@@ -1101,6 +1092,48 @@ public void testNPE() {
         assertThat( dmnResult.getMessages().stream().anyMatch( m -> m.getMessageType().equals( DMNMessageType.ERROR_EVAL_NODE ) ), is( true ) );
     }
 
+    @Test
+    public void testUnknownVariable1() {
+        DMNRuntime runtime = DMNRuntimeUtil.createRuntime( "unknown_variable1.dmn", this.getClass() );
+        DMNModel dmnModel = runtime.getModel(
+                "http://www.trisotech.com/definitions/_9105d4a6-6049-4ace-a9cd-88f18d29bc8f",
+                "Loan Recommendation - context" );
+        assertThat( dmnModel, notNullValue() );
+        assertThat( formatMessages( dmnModel.getMessages() ), dmnModel.getMessages().size(), is( 2 ) );
+        assertEquals(1, dmnModel.getMessages().stream().filter( m -> m.getMessageType().equals(DMNMessageType.ERR_COMPILING_FEEL) )
+                                                       .filter( m -> m.getMessage().contains("Unknown variable 'NonSalaryPct'") )
+                                                       .count());
+    }
+
+    @Test
+    public void testUnknownVariable2() {
+        DMNRuntime runtime = DMNRuntimeUtil.createRuntime( "unknown_variable2.dmn", this.getClass() );
+        DMNModel dmnModel = runtime.getModel(
+                "http://www.trisotech.com/definitions/_9105d4a6-6049-4ace-a9cd-88f18d29bc8f",
+                "Loan Recommendation - context" );
+        assertThat( dmnModel, notNullValue() );
+        assertThat( formatMessages( dmnModel.getMessages() ), dmnModel.getMessages().size(), is( 1 ) );
+        assertThat( dmnModel.getMessages().get( 0 ).getMessageType(), is( DMNMessageType.ERR_COMPILING_FEEL ) );
+        assertThat( dmnModel.getMessages().get( 0 ).getMessage(), containsString( "Unknown variable 'Borrower.liquidAssetsAmt'" ) );
+    }
+
+    @Test
+    public void testSingleDecisionWithContext() {
+        DMNRuntime runtime = DMNRuntimeUtil.createRuntime( "SingleDecisionWithContext.dmn", this.getClass() );
+        DMNModel dmnModel = runtime.getModel(
+                "http://www.trisotech.com/definitions/_71af58db-e1df-4b0f-aee2-48e0e8d89672",
+                "SingleDecisionWithContext" );
+        assertThat( dmnModel, notNullValue() );
+        assertThat( formatMessages( dmnModel.getMessages() ), dmnModel.hasErrors(), is( false ) );
+
+        DMNContext emptyContext = runtime.newContext();
+        DMNResult dmnResult = runtime.evaluateAll( dmnModel, emptyContext );
+
+        assertThat( formatMessages( dmnResult.getMessages() ), dmnResult.hasErrors(), is( false ) );
+        DMNContext result = dmnResult.getContext();
+        assertThat( result.get( "MyDecision" ), is( "Hello John Doe" ) );
+    }
+
     private String formatMessages(List<DMNMessage> messages) {
         return messages.stream().map( m -> m.toString() ).collect( Collectors.joining( "\n" ) );
     }

diff --git a/kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/SingleDecisionWithContext.dmn b/kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/SingleDecisionWithContext.dmn
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<semantic:definitions xmlns:semantic="http://www.omg.org/spec/DMN/20151101/dmn.xsd" xmlns="http://www.trisotech.com/definitions/_71af58db-e1df-4b0f-aee2-48e0e8d89672" xmlns:feel="http://www.omg.org/spec/FEEL/20140401" xmlns:triso="http://www.trisotech.com/2015/triso/modeling" xmlns:trisofeed="http://trisotech.com/feed" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" exporter="DMN Modeler" exporterVersion="5.1.11" id="_71af58db-e1df-4b0f-aee2-48e0e8d89672" name="SingleDecisionWithContext" namespace="http://www.trisotech.com/definitions/_71af58db-e1df-4b0f-aee2-48e0e8d89672" triso:logoChoice="Default">
+  <semantic:extensionElements/>
+  <semantic:decision id="_2ac4745e-6141-48ab-954a-2e5ed65d889e" name="MyDecision" triso:displayName="MyDecision">
+    <semantic:variable id="_86f590fd-3323-4227-820c-73134879d90e" name="MyDecision" typeRef="feel:string"/>
+    <semantic:context id="_73a9d419-641a-48c8-9d55-7d94e3ed7a15">
+      <semantic:contextEntry>
+        <semantic:variable id="_23fedcc5-b464-4adf-83fc-4702a129c1f3" name="MyPerson"/>
+        <semantic:context id="_7cc32ad5-7d10-4551-ba4f-f548df851563">
+          <semantic:contextEntry>
+            <semantic:variable id="_32fe1d12-5112-476e-957d-04d06fe9192e" name="FirstName"/>
+            <semantic:literalExpression id="_a27d192d-ee15-4b30-b3c6-b7a6050902b7">
+              <semantic:text>"John"</semantic:text>
+            </semantic:literalExpression>
+          </semantic:contextEntry>
+          <semantic:contextEntry>
+            <semantic:variable id="_bd51b6b5-f677-4c92-809f-facc9ce7b946" name="LastName"/>
+            <semantic:literalExpression id="_87e82b4a-d1a5-419c-b7cd-50e867962e8e">
+              <semantic:text>"Doe"</semantic:text>
+            </semantic:literalExpression>
+          </semantic:contextEntry>
+        </semantic:context>
+      </semantic:contextEntry>
+      <semantic:contextEntry>
+        <semantic:literalExpression id="_d2288ecc-f992-488b-b8a7-33c53a23ecf3">
+          <semantic:text>"Hello " + MyPerson.FirstName + " " + MyPerson.LastName</semantic:text>
+        </semantic:literalExpression>
+      </semantic:contextEntry>
+    </semantic:context>
+  </semantic:decision>
+</semantic:definitions>