Skip to content

Conversation

@opatrascoiu
Copy link
Contributor

@opatrascoiu opatrascoiu commented Mar 25, 2025

This PR contains fixes for #674 as per discussion in the DMN RTF:

  • hrefs contain DMN namespaces not locations
  • href must contain a namespace when the imported DRG element is not defined in the current DMN file

@StrayAlien
Copy link
Contributor

hey @opatrascoiu - a few notes so far:

////

Decision decision002 in 1158-noname-imports-03-A.dmn has an expression with a typeref of string, but the decision has no typeref. Perhaps best to remove the string typeref from the expression

7.3.1 Expression metamodel

"If an instance of Expression that defines the output of a Decision element includes a typeRef, the referenced type SHALL be the same as the type of the containing Decision element."

////

I think the variable name of bkm001 in 1158-noname-imports-03-B.dmn is not correct.  It is model_b_bkm001 but should be bkm001.  The variable name must match the BKM name I believe:  Like this:

```
    <businessKnowledgeModel name="bkm001" id="_model_b_bkm001">
    <variable name="bkm001"/>
    <encapsulatedLogic>
        <literalExpression typeRef="string">
            <text>"model_b_bkm001"</text>
        </literalExpression>
    </encapsulatedLogic>
</businessKnowledgeModel>
```

//// 

That aside, looking at some of these tests they really are no longer doing what they intended - they were written on the assumption that imports were like an 'include' and kind of blended into the current namespace.  In that world, name collisions were resolved as part of the import / compilation process.

For example, the tests in `1158-noname-imports-test-03.xml` are about testing conflicts, but, as the resective knowledge or informatoin requirements do not have a namespace, then they do not actually refer to an imported element - in the new world, they are actually local.

I'm left wondering when a conflict can occur. Take this modified example from the above test suite., It now points at an imported BKM ...

    <decision name="decision002" id="_decision002">
        <variable name="decision002"/>
        <knowledgeRequirement>
            <requiredKnowledge href="http://www.montera.com.au/spec/DMN/1158-noname-imports-03-B#_model_b_bkm001"/>
        </knowledgeRequirement>
        <literalExpression>
            <text>bkm001()</text>
        </literalExpression>
    </decision>

There IS a bkm called bkm001 in the local model .... but, as this requirement has been namespaced then surely there is no conflict?? There could only be a conflict if something like the following occurs? When two things of the same name are requirements?

    <decision name="decision002" id="_decision002">
        <variable name="decision002"/>
        <knowledgeRequirement>
            <requiredKnowledge href="http://www.montera.com.au/spec/DMN/1158-noname-imports-03-B#_model_b_bkm001"/>
        </knowledgeRequirement>
        <knowledgeRequirement>
            <requiredKnowledge href="#_model_a_bkm001"/>
        </knowledgeRequirement>
        <literalExpression>
            <text>bkm001()</text>
        </literalExpression>
    </decision>

I think this does not apply to imported types ... because they not namespaced ... they use the import name as a prefix (if I recall) ... and now there is none .... So, in that sense imported type name collisions are conceptually different to DRG element name collisions .. ?

@opatrascoiu
Copy link
Contributor Author

@StrayAlien Regarding 1158-noname-imports-03-A.dmn you are right, just pushed a fix.

Regarding 1158-noname-imports-03-B.dmn, I noticed the issue a while back and fixed in the second commit. Please update your copy.

@baldimir
Copy link
Member

@dmn-tck/contributors please review.

@baldimir
Copy link
Member

@dmn-tck/contributors please review. This is an important topic, good to be merged as soon as possible so we can move focus on 1.6.

@yesamer
Copy link
Contributor

yesamer commented Oct 21, 2025

@opatrascoiu Can you please rebase with master?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants