DMN - TCK failure: Return coerced null from decision with knowledge requirements

[gitgabrio]

TCK Failures

"compliance-level-3/0082-feel-coercion","0082-feel-coercion-test-01","decision_bkm_002","ERROR","FAILURE: 'decision_bkm_002' expected='null' but found='false'"
"compliance-level-3/0082-feel-coercion","0082-feel-coercion-test-01","invoke_001","ERROR","FAILURE: 'invoke_001' expected='null' but found='false'"
"compliance-level-3/0082-feel-coercion","0082-feel-coercion-test-01","invoke_005","ERROR","FAILURE: 'invoke_005' expected='null' but found='10'"

Model https://github.com/dmn-tck/tck/blob/master/TestCases/compliance-level-3/0082-feel-coercion/0082-feel-coercion.dmn
Test https://github.com/dmn-tck/tck/blob/master/TestCases/compliance-level-3/0082-feel-coercion/0082-feel-coercion-test-01.xml

[gitgabrio]

@baldimir @yesamer

I have the impression decision_bkm_002 test is wrong.
tNameAndAge is a ComplexType with name (String) and age (number)

    <itemDefinition name="tNameAndAge">
        <itemComponent name="name">
            <typeRef>string</typeRef>
        </itemComponent>
        <itemComponent name="age">
            <typeRef>number</typeRef>
        </itemComponent>
    </itemDefinition> 

bkm_001 is a BusinessKnowledgeModel that return true if given nameAndAge is != null, false otherwise

    <businessKnowledgeModel name="bkm_001" id="_bkm_001">
        <variable name="bkm_001"/>
        <encapsulatedLogic>
            <formalParameter name="nameAndAge" typeRef="tNameAndAge"/>
            <literalExpression>
                <text>nameAndAge != null</text>
            </literalExpression>
        </encapsulatedLogic>
    </businessKnowledgeModel>

decision_bkm_002 is a Decision that invokes _bkm_001 with an invalid nameAndAge (it miss age field) , that becomes null (a WARNING message is printed in console)

<decision name="decision_bkm_002" id="_decision_bkm_002">
        <variable name="decision_bkm_002"/>
        <knowledgeRequirement>
            <requiredKnowledge href="#_bkm_001"/>
        </knowledgeRequirement>
        <literalExpression>
            <text>bkm_001({name: "foo"})</text>
        </literalExpression>
    </decision>

Then, _bkm_001 is invoked with nameAndAge->null, and correctly return false

[gitgabrio]

@baldimir @yesamer
As previous one, I have the impression that also invoke_001 could be wrong, since it invokes the same _bkm_001 with, again, an invalid (null) nameAndAge

<decision name="invoke_001" id="_invoke_001">
        <variable name="invoke_001"/>
        <knowledgeRequirement>
            <requiredKnowledge href="#_bkm_001"/>
        </knowledgeRequirement>
        <invocation>
            <literalExpression>
                <text>bkm_001</text>
            </literalExpression>
            <binding>
                <parameter name="nameAndAge"/>
                <!-- passed incorrect param to bkm -->
                <literalExpression>
                    <text>{name: "foo"}</text>
                </literalExpression>
            </binding>
        </invocation>
    </decision>

From the test,

    <testCase id="invoke_001">
        <description>decision has invocation call to bkm passing
            non-conforming context - bkm is never invoked</description>
        <resultNode name="invoke_001" type="decision" errorResult="true">
            <expected>
                <value xsi:nil="true"/>
            </expected>
        </resultNode>
    </testCase>

it seems that in this case our engine should immediately return an error when nameAndAge can't be validated, but current implementation simply store the value as null, and to change this behavior seems very error prone

[baldimir]

There is an incorrect parameter defined, so

<text>bkm_001({name: "foo"})</text>

should resolve to null. The incorrect parameter shouldn't resolve to null and be passed to the BKM. It is a similar case in my opinion, as if it would be defined with a wrong parameter type. It should just return null and log a parameter error.

[gitgabrio]

Not sure to follow, anyway, WRT to

<text>bkm_001({name: "foo"})</text>

1. our engine detect that bkm_001 requires a tNameAndAge
2. it tries to parse {name: "foo"} as tNameAndAge, but due to missing property (age) validation fails, and translate to null
3. so, the invocation become bkm_001(null)
4. and bkm_001 in this case return false

As "wrong parameter", as per bkm definition

<businessKnowledgeModel name="bkm_001" id="_bkm_001">
        <variable name="bkm_001"/>
        <encapsulatedLogic>
            <formalParameter name="nameAndAge" typeRef="tNameAndAge"/>
            <literalExpression>
                <text>nameAndAge != null</text>
            </literalExpression>
        </encapsulatedLogic>
    </businessKnowledgeModel>

null is a perfectly valid parameter for it

AFAIK, the engine/spec has always been extremely permissive with null, i.e. whenever an expression can't be parsed to object, it is simply translated to null without any error thrown, by itself.

So, in this case, it is not clear to me how we could implement that.

[gitgabrio]

@baldimir
As reference, see Matteo comment here

at Result of invoke_001 is wrong.

So, do we have historical data of TCK tests, to see if there is a regression or that tests always failed ?

[gitgabrio]

@baldimir @yesamer
About invoke_005

This is the invoked businessKnowledgeModel:

    <businessKnowledgeModel name="bkm_005" id="_bkm_005">
        <variable name="bkm_005"/>
        <encapsulatedLogic>
            <formalParameter name="arg"/>
            <literalExpression typeRef="number">
                <text>[arg]</text>
            </literalExpression>
        </encapsulatedLogic>
    </businessKnowledgeModel>

So:

1. it receives any possible argument as variable arg (formalParameter without any specific type)
2. it change it to array (literalExpression.text)
3. it triies coerce to number (literalExpression.typeRef)

if invoked with 10
10 -> [10] -> 10

This is the tested decision

    <decision name="invoke_005" id="_invoke_005">
        <variable name="invoke_005" typeRef="number"/>
        <knowledgeRequirement>
            <requiredKnowledge href="#_bkm_005"/>
        </knowledgeRequirement>
        <invocation typeRef="number"> <!-- bkm returns number array but invocation type is a number -->
            <literalExpression>
                <text>bkm_005</text>
            </literalExpression>
            <binding>
                <parameter name="arg"/>
                <literalExpression>
                    <text>[10]</text>
                </literalExpression>
            </binding>
        </invocation>
    </decision>

1. the binding part creates an arg parameter that is it an array
2. but since the invocation.TypeRef is number, that single element array is coerced to number
3. so, the number is sent as arg parameter to bkm_005

[10] -> 10 -> bkm_005(10)

The original comment from Matteo also mention that, but I have the impression something changed in the meantime.

See Result of invoke_005 is wrong.