DMN 15b - no-name imports

[StrayAlien]

Hold onto your hats. Tests for the new "no-name" import feature of DMN 1.5b.

Import tests can be very confusing, so, I have broken these up into lots of small models with an associated test suite comprising of (generally) just one assertion.

Here is the new spec text:

Multiple imports with empty import names are allowed in the default namespace and their precedence is resolved according to their definition order. When the import name attribute is an empty string, the elements are imported in the default namespace of the model.

When a name collision occurs between an element in the default namespace and an imported element, the imported element does not replace the one already in the default namespace while the elements without name collision are imported.

Some things to note:

• "name collision" is not just any name - itemDefinitions/imports can share names with DRG elements. That has been exercised here.
• I think this should also be "id collision" as ids should be unique within a model AFAIK. So, if an imported model has an id that conflicts with the importing model then it should also not be imported (IMO). I wanted to test this, but, in strictness, the spec does not mention it. I think we should test it ... somehow.
• some test models have "dependency loops" in that a model may directly or indirectly import its own namespace. We can't really test for the loop, but, we can put it in tests as a sanity check for runtimes to make sure they don't fall over.
• tests here assert stuff about direct imports as well as imports of imports.

Each test suite has some small notes in it describing the model structure and expectations.

Comments welcome.

[gitgabrio]

Hi @StrayAlien
Thanks for the PR.
I tried to test it locally, but for some reason no "1158" get executed.
At the same time, there seems to be some issue in the validation.
Could you please check or provide suggestion ? Thanks!

[StrayAlien]

HI @gitgabrio .. hmm, that could have something to do with the test runner - whichever one it is you're using (Java?). I don't use the Java one. My understanding was that the Java test runner would run each test xml in a folder and use the 'modelName' referenced in the xml as the root model for tests. That root model may import other models in the same folder.

I've just checked the TCK and, how about that, this PR has the first test folder to contain more than one xml test file. It has sometimes been spoken about to write tests like this, but, perhaps the Java test runner is not respecting multiple test files in a folder.

[gitgabrio]

Hi @StrayAlien
not sure, anyway, my doubts are the following:

1. the TCK Validation job fails - and IINW this should not happen
2. IINW, in all other tests the file name format is
   *-test-(\\d\\d).xml
   but in this folder it is
   *-(\\d\\d\\d)-test.xml

I'm not sure if there is a "rule" for that

@baldimir @yesamer wdyt ?

[yesamer]

@gitgabrio ACK

[falko]

@saig0 FYI as we just talked about imports yesterday.

[baldimir]

I would say if the format of the filename is different than elsewhere, it should be aligned as it may cause problems for the runners.

[StrayAlien]

I can change the test files name. Though, that seems a pretty odd restriction. It could be changed to something like "\d+" and be more flexible and backwards compatible ....

[gitgabrio]

@StrayAlien
I personally have no strong opinion, but IINW until now everything here worked on that "convention", on top of which runners have been implemented.
I don't know if that convention has been discussed in the past (TBH, I guess it had, since otherwise there would not have been any reason to implement such filter 🤔 ), but if we want to change it, maybe we have to make an explicit agreement.
Beside that, there is also the TCK Validation job failure, that, again, IMO is a smell of some other problem
(please keep in mind I am very new to this environment, so I may be completely wrong)

[StrayAlien]

@gitgabrio - test names have been changed.

[gitgabrio]

@baldimir @yesamer
Could you please double check ?

[yesamer]

@gitgabrio It seems the tests are still not consumed by Java-based engines, at least in my local.

[StrayAlien]

Any more information that that? I don't use Java for DMN - fancy debugging it?

[StrayAlien]

I just tried the drools runner on TCK master - I assume that is the one your using (?). I get 121 errors. I see the import tests are failing.

I can run the validation profile locally and it works.

Here are some sample drools runner errors:

[ERROR]   [Message [id=1, kieBase=defaultKieBase, level=ERROR, path=/Users/gregm/workspace/personal/dmn/tck-fork/runners/dmn-tck-runner-drools/../../TestCases/compliance-level-3/0086-import/0086-import.dmn, line=0, column=0
   text=DMN: Import type unknown: 'https://www.omg.org/spec/DMN/20230324/MODEL/'. (Invalid FEEL syntax on the referenced expression) ], Message [id=2, kieBase=defaultKieBase, level=ERROR, path=/Users/gregm/workspace/personal/dmn/tck-fork/runners/dmn-tck-runner-drools/../../TestCases/compliance-level-3/0086-import/0086-import.dmn, line=20, column=-1
   text=DMN: Unable to resolve type reference 'myimport.tPerson' on node 'A Person' (DMN id: _9e1f6cbc-584f-41f6-8748-97f579a3df43, The listed type definition was not found) ], Message [id=3, kieBase=defaultKieBase, level=ERROR, path=/Users/gregm/workspace/personal/dmn/tck-fork/runners/dmn-tck-runner-drools/../../TestCases/compliance-level-3/0086-import/0086-import.dmn, line=28, column=-1
   text=DMN: Required Business Knowledge Model 'http://www.trisotech.com/definitions/_f27bb64b-6fc7-4e1f-9848-11ba35e0df36#_32543811-b499-4608-b784-6c6f294b1c58' not found on node 'A Decision Ctx with DT' (The referenced node was not found) ], Message [id=4, kieBase=defaultKieBase, level=ERROR, path=/Users/gregm/workspace/personal/dmn/tck-fork/runners/dmn-tck-runner-drools/../../TestCases/compliance-level-3/0086-import/0086-import.dmn, line=33, column=-1
   text=DMN: Error compiling FEEL expression 'myimport.Say Hello(A Person)' for name 'normal greeting' on node 'A Decision Ctx with DT': Unknown variable 'myimport' (DMN id: _c6977a3f-f3e0-411f-a1fc-590db1b97958, Error compiling the referenced FEEL expression) ]]

As an FYI, I think the way others may have done the imports functionality with a test runner is kind of like this:

• each test xml file in a folder is a test suite
• the test suite file references a "modelName" DMN file - which is the "entry point" model
• the entry point DMN is in the same folder
• every DMN file in the same folder could be imported directly or indirectly by the entry point model by namespace. That is, if a model has an "imports" element, the import namespace will be the namespace of one of the DMN files in the same folder.

That's it I think. I can see the Java DmnTckRunner class does collect all dmn files in the same folder as the test suite and passes them to the the "beforeTestCases" function of the vendor runner impl. The drools runner takes those and adds them as extra resources to the drools DMN runtime ....

Given that it seems to fail on master ... perhaps a drools crew member could could check it out?

[baldimir]

I guess the problem is that the common runner, that all the runners can extend, uses this pattern to match files.

tck/runners/dmn-tck-runner/src/main/java/org/omg/dmn/tck/runner/junit4/DmnTckSuite.java

Line 75 in 7c85cec

return name.matches( tcName + "-test-\\d\\d.xml" );

So either you doublecheck, that test xml files in this PR satisfy this pattern, please, or I could change that filter, if we all agree, to just take all xml files from a test folder. What do you think please? @dmn-tck/contributors

[StrayAlien]

Hi @baldimir - just pushed another bounce at renaming things. Hopefully that works now.

[opatrascoiu]

@StrayAlien Thank you for the test.

I believe there is an issue with the hrefs when DRG elements are imported from a different file. According to the spec the namespace is mandatory, regardless if the import prefix is empty or not (see 13.3.2 References within the DMN XSD).

Here is an example of an incorrect href https://github.com/StrayAlien/tck/blob/c7a65b8af372cb92cd37a4688d4ad5d0a67fae37/TestCases/compliance-level-3/1158-noname-imports/1158-noname-imports-01-A.dmn#L16

[gitgabrio]

@opatrascoiu
If you are referring to a noname import
(6.3.3 Import metamodel)

When the import name attribute is an empty string, the elements are imported in the default namespace of the
model.

"nonamed import" should have the same namespace as the importing model itself, and then I have the impression the "namespace" will be redundant (since it is the same of the importing model).
At a slightly more abstract layer, I always had the impression that the idea is to consider the imported model "exactly as" if defined inside the importing one (as much as possible), and so href with namespace seems to contradict that.

All the above, in my opinion, overrule the 12.3.2 References within the DMN XSD you pointed at.

Please correct me if I'm misunderstanding your observation or missing something else of the context

@baldimir @yesamer

[opatrascoiu]

@gitgabrio I disagree. The paragraph that you mentioned does not say explicitly that the syntactic structure of the hrefs should change. It defines the operational semantics of the imports when no prefix is needed in the feel expressions.

The section that I mentioned is about the syntax of the hrefs.

[gitgabrio]

@opatrascoiu
IINW, when an hrefs refers to element of the same model/file, the namespace is not required (since it is the same as the model itself)
Then, the noname import define that nonamed imports should have same namespace as the importing model.
It seems we had a general agreement that the idea behind noname import is what I mention above (treat noname import elements as if written in the importing model itself)
And it seems that the href spec simply was not updated to explicitly deal with that situation ("if an hrer refers to an elment in a different file but with a no-name import.....").
Beside, following your approach we would have that:

1. some href does not have the namespace of the model where they are
2. some href have the same namespace of the model in which they are (because they comes from noname imports)
3. other elements does not requires explicit reference to imported model because- being a noname import - are treated as if it is in the same model.

So, I think that in this case it should be better to apply a consistent interpretation, since spec - as whole - sometime are not very well described or "coherent"

[opatrascoiu]

@gitgabrio

DMN borrowed the reference mechanism via HREFs from XML. They refer to the physical location (the file that contains the definition) of the referred element, not the namespace.

The physical location (in a file) and adding the definition via import in the current DMN context are two different things to me.

When this Decision is referenced, e.g., by an InformationRequirement in a Decision that is defined in another file, the reference could take the following form:
<requiredDecision href=”http://www.example.org/Definitions01.xml#prebureauriskDec01”/> where
220 Decision Model and Notation (DMN), v1.6 Beta 1
“http://www.example.org/Definitions01.xml” is an URI reference to the XML document in which the “PreBureau Risk Category” Decision is defined (e.g., the value of the locationURI attribute in the corresponding Import element), and “prebureauriskDec01” is the value of the id attribute for the Decision.

[opatrascoiu]

@StrayAlien I think we need to populate the namespace attributes from the TCK schema to avoid name ambiguities and make tests more readable (e.g. there are two inputDatas with the same name , but only one is imported and populated). Here is an example of what I have in mind:

Change test 1158-noname-imports-test-01 as below

<testCases xmlns="http://www.omg.org/spec/DMN/20160719/testcase"
           . . .
           namespace="http://www.montera.com.au/spec/DMN/1158-noname-imports-01-A">
    <modelName>1158-noname-imports-01-A.dmn</modelName>
. . .
    <testCase id="003">
        . . .
        <inputNode name="model_b_input001" namespace="http://www.montera.com.au/spec/DMN/1158-noname-imports-01-B">
            <value xsi:type="xsd:string">model_b_input001</value>
        </inputNode>
        <resultNode name="decision003" type="decision">
            <expected>
                <value xsi:type="xsd:string">model_b_input001</value>
            </expected>
        </resultNode>
    </testCase>
 ...

The semantics of these attributes is as follows: If a node (inputNode or resultNode) has a namespace, theappheron this one is used to find the corresponding DRG element, otherwise the namespace at the testCases level (the namespace of the model under test / root model).

testCase has a namespace as well if we want to mix tests for models in the same test files. I personally think we should avoid mixing tests for several models in the same test file.

[StrayAlien]

Hi @opatrascoiu / @gitgabrio . Thanks for the comments. Very interesting. It's great there is some discussion on these.

Having those imported hrefs as 'local' is intentional. According to the spec:

When the import name attribute is an empty string, the elements are imported in the default namespace of the model.

IMO, they become available under the default namespace of the imported model. In a sense, the existence of the import namespace is 'erased'. That also means the those imported inputs are available in the default namespace.

I feel the intention here is really the equivalent of an old-fashioned "include" - such that the imported model becomes 'included' or 'copied into' in the importing model - in that sense, the "physical location" actually becomes the same as the importing model.

That's my take on it. If it is not the case, then I'm not too sure what " imported in the default namespace of the model" might mean in practice.

If I were calling this model via an API I would not expect to have to qualify that input data name with a namespace - because, it has become 'part' of the importing model.

So, that TCK test passing an imported input as if it was part of the importing model is intentional and there to assert same.

EDIT: I think the namespace qualification that you mention is applicable to named imports for sure.

[opatrascoiu]

@StrayAlien

Regarding the paragraph mentioned here #674 (comment)

The HREFs contain the physical location (e.g. file that contains the definition) not the namespace of the model described in that file. These are different things. DMN 1.5. states the physical location should always be present if the DRG element is not described in the same file.

[gitgabrio]

@opatrascoiu
Your original comment was

I believe there is an issue with the hrefs when DRG elements are imported from a different file. According to the spec the namespace is mandatory, regardless if the import prefix is empty or not (see 13.3.2 References within the DMN XSD).

But with that:

The HREFs contain the physical location (e.g. file that contains the definition) not the namespace of the model described in that file.

it seems you were talking of physical location.
If a real, physical location is required, then all TCK models using href with physical path ("http...") are broken, because they refer the "namespace" and not an actually existing location (and this is true for lot of other models).

But that "References within the DMN XSD" paragraph has not been modified since 1.4, and the need of physical path seems to derive from

however it can only reference an element within the same file.

that was true then, but not with no-named import.

As @StrayAlien mentioned, it seems

the intention here is really the equivalent of an old-fashioned "include"

If you think there is a real need, beside the specs that maybe are simply outdated, could you please explain ? Thanks

References

https://datatracker.ietf.org/doc/html/rfc3986

Uniform Resource Identifier (URI) is a compact sequence of
characters that identifies an abstract or physical resource.

[opatrascoiu]

@gitgabrio I checked all the DMN files in the master of the TCK project. The only ones that use the namespace instead of location (see 12.3.2 References within the DMN XSD) are the ones in 0086-import and the DMN files in this PR. All the other test cases have only one DMN model hence the HREFs don't contain a location.

Also, the testCases contain a property called modelName that is the name of the file (location) and not the name of the model in the DMN file.

We need to discuss in the next meeting how are we going to move forward on the references, both in DMN files and TCK files (references of DRG elements and inputNode and resultNode).

[StrayAlien]

This spec change was driven by Trisotech? Perhaps @SimonRinguette could chime in?

[opatrascoiu]

Just to add some context here.

A while back @StrayAlien added a test where the namespace is always specified in href, including the DRG elements defined in the same file.

This use case was addressed in DMN 1.6 https://issues.omg.org/browse/DMN16-64: the namespace for hrefs to DRG elements defined in the same DMN is optional, is not an error to have it.

[baldimir]

Based on the discussion on the meeting:

• The main difference when referencing with href is that href needs to contain a physical location and not namespace.
• We have tests in TCK repository that use namespace to reference also with hrefs. E.g. here (1). We may need to review such tests.
• The no-name import may need to be clarified or discussied in RTF meetings. Looks like the intention of no-name imports is to import only to the default namespace in the DMN context, not in the XML contex, so hrefs shouldn't be affected by this.

(1) https://github.com/dmn-tck/tck/blob/a967d577aafa2dd0e456084bf964d9e8fb6da0b5/TestCases/compliance-level-3/0091-local-hrefs/0091-local-hrefs.dmn

[baldimir]

Similar FEEL context: 10.3.2.11 Scope and context stack.

[opatrascoiu]

I believe that the ambiguity in this thread is caused by

When the import name attribute is an empty string, the elements are imported in the default namespace of the model. When a name collision occurs between an element in the default namespace and an imported element, the imported element does not replace the one already in the default namespace while the elements without name collision are imported.

Following our discussion in the meeting, I believe this means

When the import name attribute is an empty string, the elements are imported directly in the current DMN context (see 7.1 Introduction metamodel and 10.3.2.11 Scope and context stack). When a name collision occurs between an element in the current context and an imported element, the imported element does not replace the one already in the current scope context while the elements without name collision are imported.

[gitgabrio]

I think we all agree on ambiguity of explanation, but any attempt to disambiguate it, seems to me just a matter of personal opinion.
Saying that "namespace" has been used instead of "context" is a possible solution, but it also seems strange to me, since "namespace" and "context" have completely different semantics.
Just to reinforce, I'm not saying any of the given interpretation is right or wrong: I'm simply saying that with the information we currently have, it is not possible (IMO) to clearly infer the behavior.

[opatrascoiu]

@gitgabrio I think the above proposal should solve the ambiguity:

I believe that confusion was created by the following paragraph in 6.3.3 Import metamodel:

When the import name attribute is an empty string, the elements are imported in the default namespace of the model. When a name collision occurs between an element in the default namespace and an imported element, the imported element does not replace the one already in the default namespace while the elements without name collision are imported.

As @SimonRinguette, the author of the change in DMN, mentioned in the meeting:

• the proposal never intended to have an impact on the XML / DMN namespaces, by adding new elements
• there are two areas in the DMN spec that are impacted by the import constructions:
  1- The structure of the XML references to the definitions of the referenced DRG element (e.g. input or decision) described in 12.3.2 References within the DMN XSD.
  2 - The type references and the execution of the expressions in certain contexts where the prefix is used (e.g. typeRefs of variables and FEEL literal expressions). The contexts in which the execution of the expressions takes place are defined in the sections mentioned here DMN 15b - no-name imports #674 (comment).

IMO there is no ambiguity in 12.3.2 References within the DMN XSD. The proposed solution addresses the issue for 2 (execution of expressions in a certain context) and does not give the impression that the DMN / XML namespaces are mutable.

@gitgabrio Is there any other aspect of the operational semantics, apart from referencing and defining the context for the execution of the logic associated to the DRG elements, that is still ambiguous in your view?

@SimonRinguette Please let me know if the above summarizes your views accurately.

[gitgabrio]

@opatrascoiu
What concern me is mostly the kind of confusion this kind of interpretation is going to create, from user POV.
I mean, on one side, for some elements, the "same namespace rule" apply - i.e. as in the same model. In other cases, the different file rule (i.e. namespace required) apply.
In my experience, we, as developer, are able to keep this distinction in minds (but not even always), but for user, that often are not so "technical", it only is a useless nuisance.
There is not any real technical need for that:

1. that "namespace" in href does not point to a real file, but to a "label"
2. the "imported" file has to be re-solved/found anyway during model parsing, to validate all the conditions where the imported namespace can be avoided.
   So, it is just our own "convention", and probably when the proposal has been made, no-one thought of such consequence.
   So, the ambiguity will be hard-coded in the model semantic itself, IMO

[opatrascoiu]

@StrayAlien I looked at tests 1158-noname-imports-test-14.xml and 1158-noname-imports-test-15.xml. I like the intention behind them!

However, I disagree with the expected result. IMO it should be:

 <testCase id="001">
    <description>requirement of imported decision will be resolved in the context of the imported decision</description>
    <resultNode name="decision001" type="decision">
        <expected>
            <value xsi:type="xsd:string">model_b_decision003</value>
        </expected>
    </resultNode>
</testCase>

and

<testCase id="001">
    <description>fully qualified local requirement href of imported decision will be resolved in the context of the imported decision</description>
    <resultNode name="decision001" type="decision">
        <expected>
            <value xsi:type="xsd:string">model_b_decision003</value>
        </expected>
    </resultNode>
</testCase>

I think that according to sections 13.3.2 References within the DMN XSD and 10.3.2.11 Scope and context stack the hrefs are pointing to the DRG elements in the file of the imported decision.

Once we address the issue with the hrefs in the DNN files and #686 we are good to merge :)

[baldimir]

We need to review based on the latest agreement from RTF. Based on the discussion in RTF, the href reference contains namespace, not location. It may be possible the namespace is missing in some test cases in the hrefs. This needs to be worked on.